* bug#23771: Eliminating compiler warnings @ 2016-06-15 2:04 Ken Brown 2016-06-15 14:44 ` Eli Zaretskii 2016-06-15 20:24 ` Richard Stallman 0 siblings, 2 replies; 14+ messages in thread From: Ken Brown @ 2016-06-15 2:04 UTC (permalink / raw) To: 23771 [-- Attachment #1: Type: text/plain, Size: 1435 bytes --] I would like to get rid of all compiler warnings in the Cygwin builds (X11, w32, and nox). There are currently none in the X11 build, but there are a lot in the other builds. Many of the warnings in the w32 build probably occur in the MinGW build also. And all of the warnings in the nox build would occur in any build without a window system. The seven patches attached attempt to eliminate all the currently existing warnings. There is one patch for each type of warning. Since they affect other platforms besides Cygwin, I won't install them until someone has had a chance to review them. There's obviously no rush about this. Two comments: First, patch 0006-... is there because there might be a jump over an AUTO_STRING call. (This happens exactly once in the Cygwin-w32 build.) It seems stupid to have to worry about this. An alternative would be to just disable the -Wjump-misses-init warning. Second, patch 0007-... is there because I couldn't think of a reasonable way to avoid -Waddress warnings when compiling w32fns.c, w32menu.c, and menu.c in the Cygwin-w32 build. Everything I thought of would have made the code very ugly. So I simply disabled that warning for the Cygwin-w32 build, and I took the liberty of doing the same thing for the MinGW build, which I think is also affected in some cases. If someone sees a better way of eliminating those warnings, that would be preferable. Ken [-- Attachment #2: 0001-Eliminate-noreturn-warnings-if-there-s-no-window-sys.patch --] [-- Type: text/plain, Size: 2983 bytes --] From 15bda1697070a1fe97b1706fa27ac49c2439b6cb Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Mon, 13 Jun 2016 17:11:57 -0400 Subject: [PATCH 1/7] =?UTF-8?q?Eliminate=20"noreturn"=20warnings=20if=20th?= =?UTF-8?q?ere=E2=80=99s=20no=20window=20system?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/frame.c (decode_window_system_frame, check_window_system): Define new _Noreturn versions if !HAVE_WINDOW_SYSTEM. * src/frame.h: (decode_window_system_frame, check_window_system): Adjust prototypes. --- src/frame.c | 18 ++++++++++++++++-- src/frame.h | 6 ++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/frame.c b/src/frame.c index df97539..653d660 100644 --- a/src/frame.c +++ b/src/frame.c @@ -113,8 +113,6 @@ window_system_available (struct frame *f) return f ? FRAME_WINDOW_P (f) || FRAME_MSDOS_P (f) : x_display_list != NULL; } -#endif /* HAVE_WINDOW_SYSTEM */ - struct frame * decode_window_system_frame (Lisp_Object frame) { @@ -133,6 +131,22 @@ check_window_system (struct frame *f) : "Window system is not in use or not initialized"); } +#else /* not HAVE_WINDOW_SYSTEM */ + +_Noreturn void +decode_window_system_frame (Lisp_Object frame) +{ + error ("Window system is not in use"); +} + +_Noreturn void +check_window_system (struct frame *f) +{ + error ("Window system is not in use"); +} + +#endif /* not HAVE_WINDOW_SYSTEM */ + /* Return the value of frame parameter PROP in frame FRAME. */ Lisp_Object diff --git a/src/frame.h b/src/frame.h index 7d64e00..0189ca1 100644 --- a/src/frame.h +++ b/src/frame.h @@ -1102,12 +1102,13 @@ extern Lisp_Object selected_frame; extern int frame_default_tool_bar_height; #endif -extern struct frame *decode_window_system_frame (Lisp_Object); extern struct frame *decode_live_frame (Lisp_Object); extern struct frame *decode_any_frame (Lisp_Object); extern struct frame *make_initial_frame (void); extern struct frame *make_frame (bool); #ifdef HAVE_WINDOW_SYSTEM +extern void check_window_system (struct frame *); +extern struct frame *decode_window_system_frame (Lisp_Object); extern struct frame *make_minibuffer_frame (void); extern struct frame *make_frame_without_minibuffer (Lisp_Object, struct kboard *, @@ -1115,8 +1116,9 @@ extern struct frame *make_frame_without_minibuffer (Lisp_Object, extern bool window_system_available (struct frame *); #else /* not HAVE_WINDOW_SYSTEM */ #define window_system_available(f) ((void) (f), false) +extern _Noreturn void check_window_system (struct frame *); +extern _Noreturn void decode_window_system_frame (Lisp_Object); #endif /* HAVE_WINDOW_SYSTEM */ -extern void check_window_system (struct frame *); extern void frame_make_pointer_invisible (struct frame *); extern void frame_make_pointer_visible (struct frame *); extern Lisp_Object delete_frame (Lisp_Object, Lisp_Object); -- 2.8.3 [-- Attachment #3: 0002-Fix-unused-variable-compiler-warnings.patch --] [-- Type: text/plain, Size: 15577 bytes --] From 3a373d7dc22bee795519f619e0504d23fb784cac Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Tue, 14 Jun 2016 15:09:35 -0400 Subject: [PATCH 2/7] Fix "unused variable" compiler warnings * src/frame.h (FRAME_FRINGE_COLS, FRAME_TOTAL_FRINGE_WIDTH) (FRAME_LEFT_FRINGE_WIDTH, FRAME_RIGHT_FRINGE_WIDTH) (FRAME_INTERNAL_BORDER_WIDTH, FRAME_RIGHT_DIVIDER_WIDTH) (FRAME_BOTTOM_DIVIDER_WIDTH): Explicitly discard unused argument. * src/image.c [HAVE_NTGUI] (DefaultDepthOfScreen): Remove unused macro. (x_create_bitmap_from_data): Declare 'frame'. (x_create_bitmap_from_file): Don't declare unused variable dpyinfo. * src/w32fns.c (Fx_show_tip): Declare 'f'. (Fx_file_dialog): Declare 'filter_a' only if not NTGUI_UNICODE. * src/w32term.c (w32_scroll_bar_handle_click): (w32_horizontal_scroll_bar_handle_click): Declare 'f'. * src/w32term.h (FRAME_DISPLAY_INFO): Explicitly discard unused argument. * src/xdisp.c (handle_single_display_spec): Declare 'fringe_bitmap' only if HAVE_WINDOW_SYSTEM. (append_space_for_newline): Declare 'g' only if HAVE_WINDOW_SYSTEM. (Fmove_point_visually): Declare and set 'target_is_eol_p' only if HAVE_WINDOW_SYSTEM. (show_mouse_face): Declare and set 'f' and 'phys_cursor_p' only if HAVE_WINDOW_SYSTEM. (note_mode_line_or_margin_highlight): Declare and set 'cursor' only if HAVE_WINDOW_SYSTEM. (note_mouse_highlight): Declare and set 'cursor' and 'pointer' only if HAVE_WINDOW_SYSTEM. * src/xfaces.c (realize_default_face): Declare and set 'face' only if HAVE_X_WINDOWS. * src/frame.c (Ficonify_frame, Fset_frame_position): Declare and set 'f' only if HAVE_WINDOW_SYSTEM. * src/font.c (font_open_entity): Declare and set 'min_width' only if HAVE_WINDOW_SYSTEM. * src/dispextern.h (FACE_SUITABLE_FOR_ASCII_CHAR_P) (FACE_FOR_CHAR): Explicitly discard unused arguments. * src/composite.c (autocmp_chars): Declare and set 'f' only if HAVE_WINDOW_SYSTEM. --- src/composite.c | 2 ++ src/dispextern.h | 7 +++++-- src/font.c | 7 ++++++- src/frame.c | 6 ++---- src/frame.h | 14 +++++++------- src/image.c | 6 +++--- src/w32fns.c | 4 +++- src/w32term.c | 2 ++ src/w32term.h | 2 +- src/xdisp.c | 34 +++++++++++++++++++++++++++------- src/xfaces.c | 6 ++++-- 11 files changed, 62 insertions(+), 28 deletions(-) diff --git a/src/composite.c b/src/composite.c index bef1c5f..4f9758b 100644 --- a/src/composite.c +++ b/src/composite.c @@ -867,7 +867,9 @@ autocmp_chars (Lisp_Object rule, ptrdiff_t charpos, ptrdiff_t bytepos, Lisp_Object string) { ptrdiff_t count = SPECPDL_INDEX (); +#ifdef HAVE_WINDOW_SYSTEM struct frame *f = XFRAME (win->frame); +#endif Lisp_Object pos = make_number (charpos); ptrdiff_t to; ptrdiff_t pt = PT, pt_byte = PT_BYTE; diff --git a/src/dispextern.h b/src/dispextern.h index e83b7c7..987d7f8 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -1843,8 +1843,11 @@ struct face_cache #else /* not HAVE_WINDOW_SYSTEM */ -#define FACE_SUITABLE_FOR_ASCII_CHAR_P(FACE, CHAR) true -#define FACE_FOR_CHAR(F, FACE, CHAR, POS, OBJECT) ((FACE)->id) +#define FACE_SUITABLE_FOR_ASCII_CHAR_P(FACE, CHAR) \ + ((void) (FACE), (void) (CHAR), true) +#define FACE_FOR_CHAR(F, FACE, CHAR, POS, OBJECT) \ + ((void) (F), (void) (FACE), (void) (CHAR), (void) (POS), \ + (void) (OBJECT), (FACE)->id) #endif /* not HAVE_WINDOW_SYSTEM */ diff --git a/src/font.c b/src/font.c index f289891..cedb400 100644 --- a/src/font.c +++ b/src/font.c @@ -2863,7 +2863,10 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size) struct font_driver_list *driver_list; Lisp_Object objlist, size, val, font_object; struct font *font; - int min_width, height, psize; + int height, psize; +#ifdef HAVE_WINDOW_SYSTEM + int min_width; +#endif eassert (FONT_ENTITY_P (entity)); size = AREF (entity, FONT_SIZE_INDEX); @@ -2907,10 +2910,12 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size) Fcons (font_object, AREF (entity, FONT_OBJLIST_INDEX))); font = XFONT_OBJECT (font_object); +#ifdef HAVE_WINDOW_SYSTEM min_width = (font->min_width ? font->min_width : font->average_width ? font->average_width : font->space_width ? font->space_width : 1); +#endif int font_ascent, font_descent; get_font_ascent_descent (font, &font_ascent, &font_descent); diff --git a/src/frame.c b/src/frame.c index 653d660..3fcdd58 100644 --- a/src/frame.c +++ b/src/frame.c @@ -2143,13 +2143,12 @@ DEFUN ("iconify-frame", Ficonify_frame, Siconify_frame, If omitted, FRAME defaults to the currently selected frame. */) (Lisp_Object frame) { - struct frame *f = decode_live_frame (frame); - /* Don't allow minibuf_window to remain on an iconified frame. */ check_minibuf_window (frame, EQ (minibuf_window, selected_window)); /* I think this should be done with a hook. */ #ifdef HAVE_WINDOW_SYSTEM + struct frame *f = decode_live_frame (frame); if (FRAME_WINDOW_P (f)) x_iconify_frame (f); #endif @@ -3015,13 +3014,12 @@ or bottom edge of the outer frame of FRAME relative to the right or bottom edge of FRAME's display. */) (Lisp_Object frame, Lisp_Object x, Lisp_Object y) { - register struct frame *f = decode_live_frame (frame); - CHECK_TYPE_RANGED_INTEGER (int, x); CHECK_TYPE_RANGED_INTEGER (int, y); /* I think this should be done with a hook. */ #ifdef HAVE_WINDOW_SYSTEM + register struct frame *f = decode_live_frame (frame); if (FRAME_WINDOW_P (f)) x_set_offset (f, XINT (x), XINT (y), 1); #endif diff --git a/src/frame.h b/src/frame.h index 0189ca1..ea68421 100644 --- a/src/frame.h +++ b/src/frame.h @@ -1183,13 +1183,13 @@ extern Lisp_Object Vframe_list; #else /* not HAVE_WINDOW_SYSTEM */ -#define FRAME_FRINGE_COLS(F) 0 -#define FRAME_TOTAL_FRINGE_WIDTH(F) 0 -#define FRAME_LEFT_FRINGE_WIDTH(F) 0 -#define FRAME_RIGHT_FRINGE_WIDTH(F) 0 -#define FRAME_INTERNAL_BORDER_WIDTH(F) 0 -#define FRAME_RIGHT_DIVIDER_WIDTH(F) 0 -#define FRAME_BOTTOM_DIVIDER_WIDTH(F) 0 +#define FRAME_FRINGE_COLS(F) ((void) (F), 0) +#define FRAME_TOTAL_FRINGE_WIDTH(F) ((void) (F), 0) +#define FRAME_LEFT_FRINGE_WIDTH(F) ((void) (F), 0) +#define FRAME_RIGHT_FRINGE_WIDTH(F) ((void) (F), 0) +#define FRAME_INTERNAL_BORDER_WIDTH(F) ((void) (F), 0) +#define FRAME_RIGHT_DIVIDER_WIDTH(F) ((void) (F), 0) +#define FRAME_BOTTOM_DIVIDER_WIDTH(F) ((void) (F), 0) #endif /* not HAVE_WINDOW_SYSTEM */ \f diff --git a/src/image.c b/src/image.c index 657852b..88ec2b7 100644 --- a/src/image.c +++ b/src/image.c @@ -80,7 +80,6 @@ typedef struct w32_bitmap_record Bitmap_Record; #define PIX_MASK_DRAW 1 #define x_defined_color w32_defined_color -#define DefaultDepthOfScreen(screen) (one_w32_display_info.n_cbits) #endif /* HAVE_NTGUI */ @@ -223,6 +222,7 @@ x_create_bitmap_from_data (struct frame *f, char *bits, unsigned int width, unsi #endif /* HAVE_X_WINDOWS */ #ifdef HAVE_NTGUI + Lisp_Object frame UNINIT; Pixmap bitmap; bitmap = CreateBitmap (width, height, FRAME_DISPLAY_INFO (XFRAME (frame))->n_planes, @@ -272,9 +272,9 @@ x_create_bitmap_from_file (struct frame *f, Lisp_Object file) { #ifdef HAVE_NTGUI return -1; /* W32_TODO : bitmap support */ -#endif /* HAVE_NTGUI */ - +#else Display_Info *dpyinfo = FRAME_DISPLAY_INFO (f); +#endif #ifdef HAVE_NS ptrdiff_t id; diff --git a/src/w32fns.c b/src/w32fns.c index f2b438c..ff45f53 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -6888,7 +6888,7 @@ A tooltip's maximum size is specified by `x-max-tooltip-size'. Text larger than the specified size is clipped. */) (Lisp_Object string, Lisp_Object frame, Lisp_Object parms, Lisp_Object timeout, Lisp_Object dx, Lisp_Object dy) { - struct frame *tip_f; + struct frame *f, *tip_f; struct window *w; int root_x, root_y; struct buffer *old_buffer; @@ -7288,7 +7288,9 @@ value of DIR as in previous invocations; this is standard Windows behavior. */) { /* Filter index: 1: All Files, 2: Directories only */ static const wchar_t filter_w[] = L"All Files (*.*)\0*.*\0Directories\0*|*\0"; +#ifndef NTGUI_UNICODE static const char filter_a[] = "All Files (*.*)\0*.*\0Directories\0*|*\0"; +#endif Lisp_Object filename = default_filename; struct frame *f = SELECTED_FRAME (); diff --git a/src/w32term.c b/src/w32term.c index c16c8f4..649e743 100644 --- a/src/w32term.c +++ b/src/w32term.c @@ -4184,6 +4184,7 @@ w32_scroll_bar_handle_click (struct scroll_bar *bar, W32Msg *msg, int dragging = bar->dragging; SCROLLINFO si; int sb_event = LOWORD (msg->msg.wParam); + struct frame *f; si.cbSize = sizeof (si); if (sb_event == SB_THUMBTRACK) @@ -4298,6 +4299,7 @@ w32_horizontal_scroll_bar_handle_click (struct scroll_bar *bar, W32Msg *msg, int dragging = bar->dragging; SCROLLINFO si; int sb_event = LOWORD (msg->msg.wParam); + struct frame *f; si.cbSize = sizeof (si); if (sb_event == SB_THUMBTRACK) diff --git a/src/w32term.h b/src/w32term.h index e134da5..3204770 100644 --- a/src/w32term.h +++ b/src/w32term.h @@ -399,7 +399,7 @@ extern struct w32_output w32term_display; #define FRAME_BASELINE_OFFSET(f) ((f)->output_data.w32->baseline_offset) /* This gives the w32_display_info structure for the display F is on. */ -#define FRAME_DISPLAY_INFO(f) (&one_w32_display_info) +#define FRAME_DISPLAY_INFO(f) ((void) (f), (&one_w32_display_info)) /* This is the `Display *' which frame F is on. */ #define FRAME_X_DISPLAY(f) (0) diff --git a/src/xdisp.c b/src/xdisp.c index d589080..9a58238 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -5017,7 +5017,9 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object, || EQ (XCAR (spec), Qright_fringe)) && CONSP (XCDR (spec))) { +#ifdef HAVE_WINDOW_SYSTEM int fringe_bitmap; +#endif if (it) { @@ -19509,8 +19511,9 @@ append_space_for_newline (struct it *it, bool default_face_p) struct text_pos saved_pos; Lisp_Object saved_object; struct face *face; +#ifdef HAVE_WINDOW_SYSTEM struct glyph *g; - +#endif saved_object = it->object; saved_pos = it->position; @@ -21687,8 +21690,9 @@ Value is the new character position of point. */) int pt_x, target_x, pixel_width, pt_vpos; bool at_eol_p; bool overshoot_expected = false; +#ifdef HAVE_WINDOW_SYSTEM bool target_is_eol_p = false; - +#endif /* Setup the arena. */ SET_TEXT_POS (pt, PT, PT_BYTE); start_display (&it, w, pt); @@ -21802,7 +21806,9 @@ Value is the new character position of point. */) { move_it_by_lines (&it, -1); target_x = it.last_visible_x - !FRAME_WINDOW_P (it.f); +#ifdef HAVE_WINDOW_SYSTEM target_is_eol_p = true; +#endif /* Under word-wrap, we don't know the x coordinate of the last character displayed on the previous line, which immediately precedes the wrap point. To find @@ -28594,7 +28600,9 @@ static void show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw) { struct window *w = XWINDOW (hlinfo->mouse_face_window); +#ifdef HAVE_WINDOW_SYSTEM struct frame *f = XFRAME (WINDOW_FRAME (w)); +#endif if (/* If window is in the process of being destroyed, don't bother to do anything. */ @@ -28605,7 +28613,10 @@ show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw) anymore. This can happen when a window is split. */ && hlinfo->mouse_face_end_row < w->current_matrix->nrows) { +#ifdef HAVE_WINDOW_SYSTEM bool phys_cursor_on_p = w->phys_cursor_on_p; +#endif + struct glyph_row *row, *first, *last; first = MATRIX_ROW (w->current_matrix, hlinfo->mouse_face_beg_row); @@ -29697,8 +29708,8 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y, Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f); #ifdef HAVE_WINDOW_SYSTEM Display_Info *dpyinfo; -#endif Cursor cursor = No_Cursor; +#endif Lisp_Object pointer = Qnil; int dx, dy, width, height; ptrdiff_t charpos; @@ -29950,9 +29961,10 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y, && hlinfo->mouse_face_beg_row == vpos ) return; +#ifdef HAVE_WINDOW_SYSTEM if (clear_mouse_face (hlinfo)) cursor = No_Cursor; - +#endif if (!row->reversed_p) { hlinfo->mouse_face_beg_col = hpos; @@ -30017,8 +30029,10 @@ note_mouse_highlight (struct frame *f, int x, int y) enum window_part part = ON_NOTHING; Lisp_Object window; struct window *w; +#ifdef HAVE_WINDOW_SYSTEM Cursor cursor = No_Cursor; Lisp_Object pointer = Qnil; /* Takes precedence over cursor! */ +#endif struct buffer *b; /* When a menu is active, don't highlight because this looks odd. */ @@ -30201,9 +30215,9 @@ note_mouse_highlight (struct frame *f, int x, int y) && glyph->type == STRETCH_GLYPH && glyph->avoid_cursor_p)) { +#ifdef HAVE_WINDOW_SYSTEM if (clear_mouse_face (hlinfo)) cursor = No_Cursor; -#ifdef HAVE_WINDOW_SYSTEM if (FRAME_WINDOW_P (f) && NILP (pointer)) { if (area != TEXT_AREA) @@ -30256,9 +30270,10 @@ note_mouse_highlight (struct frame *f, int x, int y) same_region = coords_in_mouse_face_p (w, hpos, vpos); +#ifdef HAVE_WINDOW_SYSTEM if (same_region) cursor = No_Cursor; - +#endif /* Check mouse-face highlighting. */ if (! same_region /* If there exists an overlay with mouse-face overlapping @@ -30283,10 +30298,11 @@ note_mouse_highlight (struct frame *f, int x, int y) goto check_help_echo; hlinfo->mouse_face_overlay = overlay; +#ifdef HAVE_WINDOW_SYSTEM /* Clear the display of the old active region, if any. */ if (clear_mouse_face (hlinfo)) cursor = No_Cursor; - +#endif /* If no overlay applies, get a text property. */ if (NILP (overlay)) mouse_face = Fget_text_property (position, Qmouse_face, object); @@ -30316,7 +30332,9 @@ note_mouse_highlight (struct frame *f, int x, int y) = face_at_string_position (w, object, pos, 0, &ignore, glyph->face_id, true); show_mouse_face (hlinfo, DRAW_MOUSE_FACE); +#ifdef HAVE_WINDOW_SYSTEM cursor = No_Cursor; +#endif } else { @@ -30400,7 +30418,9 @@ note_mouse_highlight (struct frame *f, int x, int y) : XFASTINT (after), before_string, after_string, disp_string); +#ifdef HAVE_WINDOW_SYSTEM cursor = No_Cursor; +#endif } } } diff --git a/src/xfaces.c b/src/xfaces.c index de73c01..426b9ab 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -5195,10 +5195,12 @@ realize_basic_faces (struct frame *f) static bool realize_default_face (struct frame *f) { +#ifdef HAVE_X_WINDOWS struct face_cache *c = FRAME_FACE_CACHE (f); + struct face *face; +#endif Lisp_Object lface; Lisp_Object attrs[LFACE_VECTOR_SIZE]; - struct face *face; /* If the `default' face is not yet known, create it. */ lface = lface_from_face_name (f, Qdefault, false); @@ -5288,10 +5290,10 @@ realize_default_face (struct frame *f) eassert (lface_fully_specified_p (XVECTOR (lface)->contents)); check_lface (lface); memcpy (attrs, XVECTOR (lface)->contents, sizeof attrs); - face = realize_face (c, attrs, DEFAULT_FACE_ID); #ifdef HAVE_WINDOW_SYSTEM #ifdef HAVE_X_WINDOWS + face = realize_face (c, attrs, DEFAULT_FACE_ID); if (FRAME_X_P (f) && face->font != FRAME_FONT (f)) { /* This can happen when making a frame on a display that does -- 2.8.3 [-- Attachment #4: 0003-Avoid-empty-body-compiler-warnings.patch --] [-- Type: text/plain, Size: 731 bytes --] From 6cd2bafdcf9ae2fda8a377bcf5d254a09138605b Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Tue, 14 Jun 2016 18:17:57 -0400 Subject: [PATCH 3/7] Avoid "empty body" compiler warnings * src/conf_post.h (DebPrint): Add empty braces if not EMACSDEBUG. --- src/conf_post.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf_post.h b/src/conf_post.h index 4459caf..25db0c6 100644 --- a/src/conf_post.h +++ b/src/conf_post.h @@ -211,7 +211,7 @@ You lose; /* Emacs for DOS must be compiled with DJGPP */ extern void _DebPrint (const char *fmt, ...); # define DebPrint(stuff) _DebPrint stuff # else -# define DebPrint(stuff) +# define DebPrint(stuff) {} # endif #endif -- 2.8.3 [-- Attachment #5: 0004-Fix-format-compiler-warnings.patch --] [-- Type: text/plain, Size: 2608 bytes --] From 3450cf787f6b1ea3f083e147705c454d8efd4416 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Tue, 14 Jun 2016 18:52:41 -0400 Subject: [PATCH 4/7] Fix "format" compiler warnings * src/w32fns.c (w32_strerror): Use format specifier %d for sprintf argument of type int. (emacs_abort): Cast sprintf argument of type DWORD to unsigned int, and use format specifier %x, for compatibility with Cygwin. * src/w32font.c (w32_to_x_charset): Use format specifier %d for sprintf argument of type int. * src/w32term.c (x_draw_glyphless_glyph_string_foreground): Cast sprintf argument of type int to unsigned int to match %X format specifier. --- src/w32fns.c | 6 +++--- src/w32font.c | 4 ++-- src/w32term.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/w32fns.c b/src/w32fns.c index ff45f53..86ebc8f 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -8964,7 +8964,7 @@ w32_strerror (int error_no) --ret; buf[ret] = '\0'; if (!ret) - sprintf (buf, "w32 error %u", error_no); + sprintf (buf, "w32 error %d", error_no); return buf; } @@ -10333,8 +10333,8 @@ emacs_abort (void) but not on Windows 7. addr2line doesn't mind a missing "0x", but will be confused by an extra one. */ if (except_addr) - sprintf (buf, "\r\nException 0x%lx at this address:\r\n%p\r\n", - except_code, except_addr); + sprintf (buf, "\r\nException 0x%x at this address:\r\n%p\r\n", + (unsigned int) except_code, except_addr); if (stderr_fd >= 0) { if (except_addr) diff --git a/src/w32font.c b/src/w32font.c index b8884a5..4d15cff 100644 --- a/src/w32font.c +++ b/src/w32font.c @@ -1747,7 +1747,7 @@ w32_to_x_charset (int fncharset, char *matching) default: /* Encode numerical value of unknown charset. */ - sprintf (buf, "*-#%u", fncharset); + sprintf (buf, "*-#%d", fncharset); return buf; } @@ -1834,7 +1834,7 @@ w32_to_x_charset (int fncharset, char *matching) /* If no match, encode the numeric value. */ if (!best_match) { - sprintf (buf, "*-#%u", fncharset); + sprintf (buf, "*-#%d", fncharset); return buf; } diff --git a/src/w32term.c b/src/w32term.c index 649e743..4462517 100644 --- a/src/w32term.c +++ b/src/w32term.c @@ -1434,7 +1434,7 @@ x_draw_glyphless_glyph_string_foreground (struct glyph_string *s) { sprintf ((char *) buf, "%0*X", glyph->u.glyphless.ch < 0x10000 ? 4 : 6, - glyph->u.glyphless.ch); + (unsigned int) glyph->u.glyphless.ch); str = buf; } -- 2.8.3 [-- Attachment #6: 0005-Don-t-define-unneeded-function-on-Cygwin.patch --] [-- Type: text/plain, Size: 1345 bytes --] From f970bedaf0b6a22b717c8c8036612f1a642e5aa4 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Tue, 14 Jun 2016 19:01:32 -0400 Subject: [PATCH 5/7] =?UTF-8?q?Don=E2=80=99t=20define=20unneeded=20functio?= =?UTF-8?q?n=20on=20Cygwin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/w32fns.c (check_w32_winkey_state): Define and use only if WINDOWSNT. --- src/w32fns.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/w32fns.c b/src/w32fns.c index 86ebc8f..f0ae241 100644 --- a/src/w32fns.c +++ b/src/w32fns.c @@ -2406,6 +2406,7 @@ hook_w32_key (int hook, int modifier, int vkey) } } +#ifdef WINDOWSNT /* Check the current Win key pressed state. */ int check_w32_winkey_state (int vkey) @@ -2433,6 +2434,7 @@ check_w32_winkey_state (int vkey) } return 0; } +#endif /* WINDOWSNT */ /* Reset the keyboard hook state. Locking the workstation with Win-L leaves the Win key(s) "down" from the hook's point of view - the @@ -2623,8 +2625,10 @@ modifier_set (int vkey) else return (GetKeyState (vkey) & 0x1); } +#ifdef WINDOWSNT if (w32_kbdhook_active && (vkey == VK_LWIN || vkey == VK_RWIN)) return check_w32_winkey_state (vkey); +#endif if (!modifiers_recorded) return (GetKeyState (vkey) & 0x8000); -- 2.8.3 [-- Attachment #7: 0006-Avoid-jump-misses-init-compiler-warnings.patch --] [-- Type: text/plain, Size: 1011 bytes --] From a447218bb7ebbc3eec49bfee9f29575483bd16b4 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Tue, 14 Jun 2016 19:06:34 -0400 Subject: [PATCH 6/7] Avoid "jump misses init" compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/lisp.h (AUTO_STRING_WITH_LEN): Don’t initialize 'name’. --- src/lisp.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lisp.h b/src/lisp.h index 972ca33..b07bf05 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4634,8 +4634,10 @@ enum STR's value is not necessarily copied. The resulting Lisp string should not be modified or made visible to user code. */ +/* Avoid initializing NAME to prevent "jump-misses-init" compiler + warnings. */ #define AUTO_STRING_WITH_LEN(name, str, len) \ - Lisp_Object name = \ + Lisp_Object name; name = \ (USE_STACK_STRING \ ? (make_lisp_ptr \ ((&(union Aligned_String) \ -- 2.8.3 [-- Attachment #8: 0007-Avoid-address-compiler-warnings.patch --] [-- Type: text/plain, Size: 948 bytes --] From fffe83061fecd26f5b534f47148ebc046cb03366 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbrown@cornell.edu> Date: Tue, 14 Jun 2016 21:29:35 -0400 Subject: [PATCH 7/7] Avoid "address" compiler warnings * configure.ac: Disable the -Wno-address compiler warning for the MinGW and Cygwin-w32 builds. --- configure.ac | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/configure.ac b/configure.ac index 2674806..3565905 100644 --- a/configure.ac +++ b/configure.ac @@ -995,6 +995,11 @@ AC_DEFUN gl_WARN_ADD([-Wno-pointer-sign]) fi + # This causes too much noise in the MinGW and Cygwin-w32 builds. + if test $opsys = mingw32 || (test $opsys = cygwin && test $with_w32 = yes); then + gl_WARN_ADD([-Wno-address]) + fi + AC_DEFINE([GCC_LINT], [1], [Define to 1 if --enable-gcc-warnings.]) AC_DEFINE([GNULIB_PORTCHECK], [1], [enable some gnulib portability checks]) AH_VERBATIM([GNULIB_PORTCHECK_FORTIFY_SOURCE], -- 2.8.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-15 2:04 bug#23771: Eliminating compiler warnings Ken Brown @ 2016-06-15 14:44 ` Eli Zaretskii 2016-06-16 1:38 ` Ken Brown 2016-06-15 20:24 ` Richard Stallman 1 sibling, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2016-06-15 14:44 UTC (permalink / raw) To: Ken Brown; +Cc: 23771 > From: Ken Brown <kbrown@cornell.edu> > Date: Tue, 14 Jun 2016 22:04:28 -0400 > > I would like to get rid of all compiler warnings in the Cygwin builds (X11, w32, and nox). There are currently none in the X11 build, but there are a lot in the other builds. Many of the warnings in the w32 build probably occur in the MinGW build also. And all of the warnings in the nox build would occur in any build without a window system. > > The seven patches attached attempt to eliminate all the currently existing warnings. There is one patch for each type of warning. Since they affect other platforms besides Cygwin, I won't install them until someone has had a chance to review them. There's obviously no rush about this. Thanks. Some comments below. Btw, it would have helped if you'd show examples of warnings that eliminated by each patch in the set; for some of them, it's easy to guess, others not so easy. I hope I did right. > Two comments: First, patch 0006-... is there because there might be a jump over an AUTO_STRING call. (This happens exactly once in the Cygwin-w32 build.) It seems stupid to have to worry about this. An alternative would be to just disable the -Wjump-misses-init warning. The warning is IMO important, it's just too bad GCC has false positives with it. > Second, patch 0007-... is there because I couldn't think of a reasonable way to avoid -Waddress warnings when compiling w32fns.c, w32menu.c, and menu.c in the Cygwin-w32 build. Everything I thought of would have made the code very ugly. So I simply disabled that warning for the Cygwin-w32 build, and I took the liberty of doing the same thing for the MinGW build, which I think is also affected in some cases. If someone sees a better way of eliminating those warnings, that would be preferable. What warnings does that option produce? I'm not sure I've seen any warnings about addresses, but maybe I misunderstand the nature of the warning. What follows is specific comments to some hunks. > +#else /* not HAVE_WINDOW_SYSTEM */ > + > +_Noreturn void > +decode_window_system_frame (Lisp_Object frame) > +{ > + error ("Window system is not in use"); > +} > + > +_Noreturn void > +check_window_system (struct frame *f) > +{ > + error ("Window system is not in use"); > +} > + > +#endif /* not HAVE_WINDOW_SYSTEM */ What kind of warnings do you get without these changes? I don't understand why this is needed. > --- a/src/font.c > +++ b/src/font.c > @@ -2863,7 +2863,10 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size) > struct font_driver_list *driver_list; > Lisp_Object objlist, size, val, font_object; > struct font *font; > - int min_width, height, psize; > + int height, psize; > +#ifdef HAVE_WINDOW_SYSTEM > + int min_width; > +#endif > > eassert (FONT_ENTITY_P (entity)); > size = AREF (entity, FONT_SIZE_INDEX); > @@ -2907,10 +2910,12 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size) > Fcons (font_object, AREF (entity, FONT_OBJLIST_INDEX))); > > font = XFONT_OBJECT (font_object); > +#ifdef HAVE_WINDOW_SYSTEM > min_width = (font->min_width ? font->min_width > : font->average_width ? font->average_width > : font->space_width ? font->space_width > : 1); > +#endif Why not move the declaration of min_width and the calculation of its value inside the #ifdef that uses it? Then you'd have eliminated 2 #ifdef's. > --- a/src/frame.c > +++ b/src/frame.c > @@ -2143,13 +2143,12 @@ DEFUN ("iconify-frame", Ficonify_frame, Siconify_frame, > If omitted, FRAME defaults to the currently selected frame. */) > (Lisp_Object frame) > { > - struct frame *f = decode_live_frame (frame); > - > /* Don't allow minibuf_window to remain on an iconified frame. */ > check_minibuf_window (frame, EQ (minibuf_window, selected_window)); > > /* I think this should be done with a hook. */ > #ifdef HAVE_WINDOW_SYSTEM > + struct frame *f = decode_live_frame (frame); > if (FRAME_WINDOW_P (f)) > x_iconify_frame (f); > #endif I understand the motivation, but this change has a downside: it changes the order of validity check of the function arguments, and it completely removes the check of the FRAME argument in the builds without X. So I don't think we should make this change this way. We could do something like this instead: #ifdef HAVE_WINDOW_SYSTEM struct frame *f = decode_live_frame (frame); #else (void) decode_live_frame (frame); #endif Or we could work around the warning in some other way, or even live with it. > @@ -3015,13 +3014,12 @@ or bottom edge of the outer frame of FRAME relative to the right or > bottom edge of FRAME's display. */) > (Lisp_Object frame, Lisp_Object x, Lisp_Object y) > { > - register struct frame *f = decode_live_frame (frame); > - > CHECK_TYPE_RANGED_INTEGER (int, x); > CHECK_TYPE_RANGED_INTEGER (int, y); > /* I think this should be done with a hook. */ > #ifdef HAVE_WINDOW_SYSTEM > + register struct frame *f = decode_live_frame (frame); > if (FRAME_WINDOW_P (f)) > x_set_offset (f, XINT (x), XINT (y), 1); > #endif Same comment here: the order of checking the arguments is important to keep on all frames in all builds. > --- a/src/w32fns.c > +++ b/src/w32fns.c > @@ -6888,7 +6888,7 @@ A tooltip's maximum size is specified by `x-max-tooltip-size'. > Text larger than the specified size is clipped. */) > (Lisp_Object string, Lisp_Object frame, Lisp_Object parms, Lisp_Object timeout, Lisp_Object dx, Lisp_Object dy) > { > - struct frame *tip_f; > + struct frame *f, *tip_f; Please add comments explaining why 'f' is needed (here and elsewhere). Someone who doesn't remember the w32 definition of FRAME_DISPLAY_INFO will stumble on this one. > +#ifdef HAVE_WINDOW_SYSTEM > struct glyph *g; > - > +#endif Better moved inside the #ifdef where it's used, no? (And please keep the empty line between declarations and code.) > +#ifdef HAVE_WINDOW_SYSTEM > if (clear_mouse_face (hlinfo)) > cursor = No_Cursor; > - > +#endif This and other similar hunks that ifdef away clear_mouse_face are incorrect: Emacs supports mouse highlight on TTY frames (if the mouse is supported, e.g. via GPM or on MS-Windows/MS-DOS), so we must call clear_mouse_face there as well. So any warnings you have here need to be solved in some other way. I would hate to re-introduce HAVE_MOUSE, having worked so hard in the past on removing it, so perhaps show the warnings you get, and let's take it from there. > --- a/src/xfaces.c > +++ b/src/xfaces.c > @@ -5195,10 +5195,12 @@ realize_basic_faces (struct frame *f) > static bool > realize_default_face (struct frame *f) > { > +#ifdef HAVE_X_WINDOWS > struct face_cache *c = FRAME_FACE_CACHE (f); > + struct face *face; > +#endif > Lisp_Object lface; > Lisp_Object attrs[LFACE_VECTOR_SIZE]; > - struct face *face; > > /* If the `default' face is not yet known, create it. */ > lface = lface_from_face_name (f, Qdefault, false); > @@ -5288,10 +5290,10 @@ realize_default_face (struct frame *f) > eassert (lface_fully_specified_p (XVECTOR (lface)->contents)); > check_lface (lface); > memcpy (attrs, XVECTOR (lface)->contents, sizeof attrs); > - face = realize_face (c, attrs, DEFAULT_FACE_ID); > > #ifdef HAVE_WINDOW_SYSTEM > #ifdef HAVE_X_WINDOWS > + face = realize_face (c, attrs, DEFAULT_FACE_ID); > if (FRAME_X_P (f) && face->font != FRAME_FONT (f)) > { This also looks wrong: the call to realize_face is NOT X-specific, see the body of that function. The value returned by realize_face is indeed used only on X, but the real effect of the call is by side effect: it adds the new face to the frame's face cache. One possible way of getting rid of the warning is something like this: #ifdef HAVE_X_WINDOWS face = realize_face (c, attrs, DEFAULT_FACE_ID); #else (void) realize_face (c, attrs, DEFAULT_FACE_ID); #endif > --- a/src/conf_post.h > +++ b/src/conf_post.h > @@ -211,7 +211,7 @@ You lose; /* Emacs for DOS must be compiled with DJGPP */ > extern void _DebPrint (const char *fmt, ...); > # define DebPrint(stuff) _DebPrint stuff > # else > -# define DebPrint(stuff) > +# define DebPrint(stuff) {} > # endif > #endif Yuck! Can we simply not use the "empty body" warning option? When is it important to flag an empty body of a function? The rest looks OK to me, thanks (but I didn't try to compile with them). ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-15 14:44 ` Eli Zaretskii @ 2016-06-16 1:38 ` Ken Brown 2016-06-16 15:14 ` Eli Zaretskii 2016-06-22 1:12 ` Paul Eggert 0 siblings, 2 replies; 14+ messages in thread From: Ken Brown @ 2016-06-16 1:38 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 23771, Paul Eggert On 6/15/2016 10:44 AM, Eli Zaretskii wrote: >> From: Ken Brown <kbrown@cornell.edu> > Btw, it would have helped if you'd show examples of warnings that > eliminated by each patch in the set; for some of them, it's easy to > guess, others not so easy. I hope I did right. Thanks for the review. Sorry, I should have thought of giving examples of the warnings. >> Two comments: First, patch 0006-... is there because there might be a jump over an AUTO_STRING call. (This happens exactly once in the Cygwin-w32 build.) It seems stupid to have to worry about this. An alternative would be to just disable the -Wjump-misses-init warning. > > The warning is IMO important, it's just too bad GCC has false > positives with it. OK. >> Second, patch 0007-... is there because I couldn't think of a reasonable way to avoid -Waddress warnings when compiling w32fns.c, w32menu.c, and menu.c in the Cygwin-w32 build. Everything I thought of would have made the code very ugly. So I simply disabled that warning for the Cygwin-w32 build, and I took the liberty of doing the same thing for the MinGW build, which I think is also affected in some cases. If someone sees a better way of eliminating those warnings, that would be preferable. > > What warnings does that option produce? I'm not sure I've seen any > warnings about addresses, but maybe I misunderstand the nature of the > warning. Here's a typical example: ../../master/src/menu.c: In function ‘digest_single_submenu’: ../../master/src/menu.c:46:30: warning: the address of ‘AppendMenuW’ will always evaluate as ‘true’ [-Waddress] # define unicode_append_menu AppendMenuW ^ ../../master/src/menu.c:691:9: note: in expansion of macro ‘unicode_append_menu’ if (unicode_append_menu) ^ ../../master/src/menu.c:46:30: warning: the address of ‘AppendMenuW’ will always evaluate as ‘true’ [-Waddress] # define unicode_append_menu AppendMenuW ^ ../../master/src/menu.c:767:9: note: in expansion of macro ‘unicode_append_menu’ if (unicode_append_menu) ^ > What follows is specific comments to some hunks. > >> +#else /* not HAVE_WINDOW_SYSTEM */ >> + >> +_Noreturn void >> +decode_window_system_frame (Lisp_Object frame) >> +{ >> + error ("Window system is not in use"); >> +} >> + >> +_Noreturn void >> +check_window_system (struct frame *f) >> +{ >> + error ("Window system is not in use"); >> +} >> + >> +#endif /* not HAVE_WINDOW_SYSTEM */ > > What kind of warnings do you get without these changes? I don't > understand why this is needed. A build with no windows system yields these warnings: ../../warnings/src/frame.c: In function ‘decode_window_system_frame’: ../../warnings/src/frame.c:119:1: warning: function might be candidate for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn] decode_window_system_frame (Lisp_Object frame) ^ ../../warnings/src/frame.c: In function ‘check_window_system’: ../../warnings/src/frame.c:129:1: warning: function might be candidate for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn] check_window_system (struct frame *f) ^ >> --- a/src/font.c >> +++ b/src/font.c >> @@ -2863,7 +2863,10 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size) >> struct font_driver_list *driver_list; >> Lisp_Object objlist, size, val, font_object; >> struct font *font; >> - int min_width, height, psize; >> + int height, psize; >> +#ifdef HAVE_WINDOW_SYSTEM >> + int min_width; >> +#endif >> >> eassert (FONT_ENTITY_P (entity)); >> size = AREF (entity, FONT_SIZE_INDEX); >> @@ -2907,10 +2910,12 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size) >> Fcons (font_object, AREF (entity, FONT_OBJLIST_INDEX))); >> >> font = XFONT_OBJECT (font_object); >> +#ifdef HAVE_WINDOW_SYSTEM >> min_width = (font->min_width ? font->min_width >> : font->average_width ? font->average_width >> : font->space_width ? font->space_width >> : 1); >> +#endif > > Why not move the declaration of min_width and the calculation of its > value inside the #ifdef that uses it? Then you'd have eliminated 2 > #ifdef's. Good idea. I'll do that. >> --- a/src/frame.c >> +++ b/src/frame.c >> @@ -2143,13 +2143,12 @@ DEFUN ("iconify-frame", Ficonify_frame, Siconify_frame, >> If omitted, FRAME defaults to the currently selected frame. */) >> (Lisp_Object frame) >> { >> - struct frame *f = decode_live_frame (frame); >> - >> /* Don't allow minibuf_window to remain on an iconified frame. */ >> check_minibuf_window (frame, EQ (minibuf_window, selected_window)); >> >> /* I think this should be done with a hook. */ >> #ifdef HAVE_WINDOW_SYSTEM >> + struct frame *f = decode_live_frame (frame); >> if (FRAME_WINDOW_P (f)) >> x_iconify_frame (f); >> #endif > > I understand the motivation, but this change has a downside: it > changes the order of validity check of the function arguments, and it > completely removes the check of the FRAME argument in the builds > without X. So I don't think we should make this change this way. Yes, I was very careless about that. I'm glad you caught it. > We could do something like this instead: > > #ifdef HAVE_WINDOW_SYSTEM > struct frame *f = decode_live_frame (frame); > #else > (void) decode_live_frame (frame); > #endif > > Or we could work around the warning in some other way, or even live > with it. I'm now thinking, especially in view of Richard's comments, that maybe we should just live with the "unused variable" warnings that can't be avoided without cluttering the code. >> @@ -3015,13 +3014,12 @@ or bottom edge of the outer frame of FRAME relative to the right or >> bottom edge of FRAME's display. */) >> (Lisp_Object frame, Lisp_Object x, Lisp_Object y) >> { >> - register struct frame *f = decode_live_frame (frame); >> - >> CHECK_TYPE_RANGED_INTEGER (int, x); >> CHECK_TYPE_RANGED_INTEGER (int, y); > >> /* I think this should be done with a hook. */ >> #ifdef HAVE_WINDOW_SYSTEM >> + register struct frame *f = decode_live_frame (frame); >> if (FRAME_WINDOW_P (f)) >> x_set_offset (f, XINT (x), XINT (y), 1); >> #endif > > Same comment here: the order of checking the arguments is important to > keep on all frames in all builds. Yes, more carelessness on my part. >> --- a/src/w32fns.c >> +++ b/src/w32fns.c >> @@ -6888,7 +6888,7 @@ A tooltip's maximum size is specified by `x-max-tooltip-size'. >> Text larger than the specified size is clipped. */) >> (Lisp_Object string, Lisp_Object frame, Lisp_Object parms, Lisp_Object timeout, Lisp_Object dx, Lisp_Object dy) >> { >> - struct frame *tip_f; >> + struct frame *f, *tip_f; > > Please add comments explaining why 'f' is needed (here and > elsewhere). Someone who doesn't remember the w32 definition of > FRAME_DISPLAY_INFO will stumble on this one. OK >> +#ifdef HAVE_WINDOW_SYSTEM >> struct glyph *g; >> - >> +#endif > > Better moved inside the #ifdef where it's used, no? Yes. >> +#ifdef HAVE_WINDOW_SYSTEM >> if (clear_mouse_face (hlinfo)) >> cursor = No_Cursor; >> - >> +#endif > > This and other similar hunks that ifdef away clear_mouse_face are > incorrect: Emacs supports mouse highlight on TTY frames (if the mouse > is supported, e.g. via GPM or on MS-Windows/MS-DOS), so we must call > clear_mouse_face there as well. The same kind of carelessness yet again. (I was just trying to get rid of the warning that 'cursor' is unused, but I obviously threw away too much.) I'll fix all these and resend the patch. >> --- a/src/conf_post.h >> +++ b/src/conf_post.h >> @@ -211,7 +211,7 @@ You lose; /* Emacs for DOS must be compiled with DJGPP */ >> extern void _DebPrint (const char *fmt, ...); >> # define DebPrint(stuff) _DebPrint stuff >> # else >> -# define DebPrint(stuff) >> +# define DebPrint(stuff) {} >> # endif >> #endif > > Yuck! Can we simply not use the "empty body" warning option? When is > it important to flag an empty body of a function? Here's a typical example: Code like this: if (!f->output_data.w32->asked_for_visible) DebPrint (("frame %p (%s) reexposed by WM_PAINT\n", f, SDATA (f->name))); leads to this warning (if EMACSDEBUG is not defined): ../../warnings/src/w32term.c: In function ‘w32_read_socket’: ../../warnings/src/w32term.c:4613:28: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] SDATA (f->name))); ^ But I'd be fine with just disabling this warning, at least for the w32 builds. Paul, do you think it's important to keep this warning? Thanks again for the review. Ken ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-16 1:38 ` Ken Brown @ 2016-06-16 15:14 ` Eli Zaretskii 2016-06-16 15:50 ` Paul Eggert 2016-06-21 3:11 ` Ken Brown 2016-06-22 1:12 ` Paul Eggert 1 sibling, 2 replies; 14+ messages in thread From: Eli Zaretskii @ 2016-06-16 15:14 UTC (permalink / raw) To: Ken Brown; +Cc: 23771, eggert > Cc: 23771@debbugs.gnu.org, Paul Eggert <eggert@cs.ucla.edu> > From: Ken Brown <kbrown@cornell.edu> > Date: Wed, 15 Jun 2016 21:38:19 -0400 > > > What warnings does that option produce? I'm not sure I've seen any > > warnings about addresses, but maybe I misunderstand the nature of the > > warning. > > Here's a typical example: > > ../../master/src/menu.c: In function ‘digest_single_submenu’: > ../../master/src/menu.c:46:30: warning: the address of ‘AppendMenuW’ > will always evaluate as ‘true’ [-Waddress] > # define unicode_append_menu AppendMenuW > ^ > ../../master/src/menu.c:691:9: note: in expansion of macro > ‘unicode_append_menu’ > if (unicode_append_menu) > ^ For this one, I'd suggest to make unicode_append_menu a variable in the Cygwin build as well, and then do this: AppendMenuW_Proc unicode_append_menu = AppendMenuW; > >> +#else /* not HAVE_WINDOW_SYSTEM */ > >> + > >> +_Noreturn void > >> +decode_window_system_frame (Lisp_Object frame) > >> +{ > >> + error ("Window system is not in use"); > >> +} > >> + > >> +_Noreturn void > >> +check_window_system (struct frame *f) > >> +{ > >> + error ("Window system is not in use"); > >> +} > >> + > >> +#endif /* not HAVE_WINDOW_SYSTEM */ > > > > What kind of warnings do you get without these changes? I don't > > understand why this is needed. > > A build with no windows system yields these warnings: > > ../../warnings/src/frame.c: In function ‘decode_window_system_frame’: > ../../warnings/src/frame.c:119:1: warning: function might be candidate > for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn] > decode_window_system_frame (Lisp_Object frame) > ^ How about making check_window_system compile in the no-X builds? > >> --- a/src/conf_post.h > >> +++ b/src/conf_post.h > >> @@ -211,7 +211,7 @@ You lose; /* Emacs for DOS must be compiled with DJGPP */ > >> extern void _DebPrint (const char *fmt, ...); > >> # define DebPrint(stuff) _DebPrint stuff > >> # else > >> -# define DebPrint(stuff) > >> +# define DebPrint(stuff) {} > >> # endif > >> #endif > > > > Yuck! Can we simply not use the "empty body" warning option? When is > > it important to flag an empty body of a function? > > Here's a typical example: > > Code like this: > > if (!f->output_data.w32->asked_for_visible) > DebPrint (("frame %p (%s) reexposed by WM_PAINT\n", f, > SDATA (f->name))); > > leads to this warning (if EMACSDEBUG is not defined): > > ../../warnings/src/w32term.c: In function ‘w32_read_socket’: > ../../warnings/src/w32term.c:4613:28: warning: suggest braces around > empty body in an ‘if’ statement [-Wempty-body] > SDATA (f->name))); > ^ One way of fixing this is to add those braces. > But I'd be fine with just disabling this warning, at least for the w32 > builds. Right. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-16 15:14 ` Eli Zaretskii @ 2016-06-16 15:50 ` Paul Eggert 2016-06-21 3:11 ` Ken Brown 1 sibling, 0 replies; 14+ messages in thread From: Paul Eggert @ 2016-06-16 15:50 UTC (permalink / raw) To: Eli Zaretskii, Ken Brown; +Cc: 23771 Eli Zaretskii wrote: > One way of fixing this is to add those braces. The accepted way to fix this is to use a do-while. See, e.g., the definition of DEFVAR_LISP in lisp.h. The warning is helpful to catch stray semicolons, e.g., 'if (n == 0); ...'. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-16 15:14 ` Eli Zaretskii 2016-06-16 15:50 ` Paul Eggert @ 2016-06-21 3:11 ` Ken Brown 1 sibling, 0 replies; 14+ messages in thread From: Ken Brown @ 2016-06-21 3:11 UTC (permalink / raw) To: Eli Zaretskii; +Cc: eggert, 23771-done Thanks for all the suggestions and corrections. I've fixed everything (I hope) and pushed to master. Closing. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-16 1:38 ` Ken Brown 2016-06-16 15:14 ` Eli Zaretskii @ 2016-06-22 1:12 ` Paul Eggert 2016-06-22 6:10 ` martin rudalics ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Paul Eggert @ 2016-06-22 1:12 UTC (permalink / raw) To: Ken Brown; +Cc: 23771 [-- Attachment #1: Type: text/plain, Size: 613 bytes --] Thanks for all the work in improving static checking for Cygwin builds. I just now checked the attached patch into master, which I hope improves on it. I tested it on Fedora (both with and without window systems). I had one problem with the recent changes, in that they suppressed all warnings about jumps over AUTO_STRING calls. That's pretty drastic, as the warnings are typically useful, so the attached patch reverts that. Can you let me know which call needs the warning suppressed in the Cygwin-specific code in the new master? I can suggest something which disables the warning just for that call. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Improve-without-x-GCC-pacification.patch --] [-- Type: text/x-patch; name="0001-Improve-without-x-GCC-pacification.patch", Size: 31178 bytes --] From 984f3658a73a1a970b417a7544c97eae2c236d57 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 22 Jun 2016 03:04:16 +0200 Subject: [PATCH] Improve --without-x GCC pacification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/composite.c (autocmp_chars): * src/conf_post.h (DebPrint) [HAVE_NTGUI && !DebPrint && !EMACSDEBUG]: Use simpler ((void) 0) for no-op expression returning void. * src/dispextern.h [HAVE_WINDOW_SYSTEM]: Include fontset.h, for face_for_char. (FACE_SUITABLE_FOR_ASCII_CHAR_P, FACE_FOR_CHAR): Now inline functions instead of macros. This avoids the need for all those casts to void. (FACE_SUITABLE_FOR_ASCII_CHAR_P): Omit 2nd (unused) arg. All uses changed. * src/frame.c (Ficonify_frame, Fset_frame_position): * src/xdisp.c (Fmove_point_visually, show_mouse_face): * src/xdisp.c (note_mode_line_or_margin_highlight) (note_mouse_highlight): Assume HAVE_WINDOW_SYSTEM for simplicity, since the code should now work either way without generating warnings. * src/frame.c (display_available) [HAVE_WINDOW_SYSTEM]: New function. (window_system_available) [HAVE_WINDOW_SYSTEM]: Move to frame.h. (decode_window_system_frame): Use check_window_system instead of rolling the code ourself. Return needed only if HAVE_WINDOW_SYSTEM. (decode_window_system_frame, check_window_system): Merge the HAVE_WINDOW_SYSTEM and !HAVE_WINDOW_SYSTEM versions into one. * src/frame.c (Ficonify_frame, Fset_frame_position): * src/xdisp.c (show_mouse_face, define_frame_cursor1) (note_mouse_highlight): Narrow the scope of the HAVE_WINDOW_SYSTEM #ifdef; this is a better way to pacify GCC. * src/xdisp.c (x_set_left_fringe, x_set_right_fringe) (x_set_right_divider_width, x_set_bottom_divider_width): * src/xfns.c (x_set_internal_border_width): Don’t use what are now function calls as lvalues. * src/frame.h (WINDOW_SYSTEM_RETURN): New macro. (decode_window_system_frame, check_window_system): Use it, to avoid the need for duplicate declarations. (window_system_available): Now an inline function. (display_available): New decl. (frame_dimension): New inline function. (FRAME_FRINGE_COLS, FRAME_LEFT_FRINGE_WIDTH) (FRAME_RIGHT_FRINGE_WIDTH, FRAME_TOTAL_FRINGE_WIDTH) (FRAME_INTERNAL_BORDER_WIDTH, FRAME_RIGHT_DIVIDER_WIDTH) (FRAME_BOTTOM_DIVIDER_WIDTH): Use it, to avoid the need for duplicate definitions. Now inline functions instead of macros. * src/gnutls.c (gnutls_log_function2i): Remove. * src/gnutls.h (GNUTLS_LOG2i): Use ‘message’ directly. This avoids complaints about gnutls_log_function2i being defined and not used on older platforms that do not need to call GNUTLS_LOG2i. * src/image.c (DefaultDepthOfScreen) [0]: Remove unused macro. * src/lisp.h (AUTO_STRING_WITH_LEN): Revert change from ‘type id = expr’ to ‘type id; id = expr’, as this would suppress valid jump-misses-init diagnostics. Let’s find a better way to address the problem. * src/vm-limit.c (__MALLOC_HOOK_VOLATILE): Define only if needed. * src/xdisp.c (handle_single_display_spec): Simplify fringe_bitmap computation. (define_frame_cursor1): Do nothing unless in a window system. All callers changed and simplified. * src/xfaces.c (realize_default_face): Use a simpler way to pacify GCC when a return value is not used on some platforms. --- src/composite.c | 6 ---- src/conf_post.h | 3 +- src/dispextern.h | 40 ++++++++++++----------- src/frame.c | 75 ++++++++++++++++++------------------------- src/frame.h | 96 ++++++++++++++++++++++++++++++++++++++------------------ src/gnutls.c | 7 ----- src/gnutls.h | 2 +- src/image.c | 3 -- src/lisp.h | 4 +-- src/vm-limit.c | 6 ++-- src/xdisp.c | 78 +++++++++++---------------------------------- src/xfaces.c | 12 +++---- src/xfns.c | 2 +- 13 files changed, 148 insertions(+), 186 deletions(-) diff --git a/src/composite.c b/src/composite.c index 5696e3e..8aa6974 100644 --- a/src/composite.c +++ b/src/composite.c @@ -867,11 +867,7 @@ autocmp_chars (Lisp_Object rule, ptrdiff_t charpos, ptrdiff_t bytepos, Lisp_Object string) { ptrdiff_t count = SPECPDL_INDEX (); -#ifdef HAVE_WINDOW_SYSTEM struct frame *f = XFRAME (win->frame); -#else - (void) XFRAME (win->frame); -#endif Lisp_Object pos = make_number (charpos); ptrdiff_t to; ptrdiff_t pt = PT, pt_byte = PT_BYTE; @@ -895,7 +891,6 @@ autocmp_chars (Lisp_Object rule, ptrdiff_t charpos, ptrdiff_t bytepos, if (len <= 0) return unbind_to (count, Qnil); to = limit = charpos + len; -#ifdef HAVE_WINDOW_SYSTEM if (FRAME_WINDOW_P (f)) { font_object = font_range (charpos, bytepos, &to, win, face, string); @@ -906,7 +901,6 @@ autocmp_chars (Lisp_Object rule, ptrdiff_t charpos, ptrdiff_t bytepos, return unbind_to (count, Qnil); } else -#endif /* not HAVE_WINDOW_SYSTEM */ font_object = win->frame; lgstring = Fcomposition_get_gstring (pos, make_number (to), font_object, string); diff --git a/src/conf_post.h b/src/conf_post.h index 431b7a9..7aa5bae 100644 --- a/src/conf_post.h +++ b/src/conf_post.h @@ -211,8 +211,7 @@ You lose; /* Emacs for DOS must be compiled with DJGPP */ extern void _DebPrint (const char *fmt, ...); # define DebPrint(stuff) _DebPrint stuff # else -/* Avoid compiler warnings about empty body of 'if' statement. */ -# define DebPrint(stuff) do {} while (false) +# define DebPrint(stuff) ((void) 0) # endif #endif diff --git a/src/dispextern.h b/src/dispextern.h index 987d7f8..d0fc3b2 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -82,6 +82,7 @@ typedef XImagePtr XImagePtr_or_DC; #ifdef HAVE_WINDOW_SYSTEM # include <time.h> +# include "fontset.h" #endif #ifndef HAVE_WINDOW_SYSTEM @@ -1825,31 +1826,32 @@ struct face_cache ? FACE_FROM_ID (F, ID) \ : NULL) +/* True if FACE is suitable for displaying ASCII characters. */ +INLINE bool +FACE_SUITABLE_FOR_ASCII_CHAR_P (struct face *face) +{ #ifdef HAVE_WINDOW_SYSTEM - -/* Non-zero if FACE is suitable for displaying character CHAR. */ - -#define FACE_SUITABLE_FOR_ASCII_CHAR_P(FACE, CHAR) \ - ((FACE) == (FACE)->ascii_face) + return face == face->ascii_face; +#else + return true; +#endif +} /* Return the id of the realized face on frame F that is like the face - FACE, but is suitable for displaying character CHAR at buffer or + FACE, but is suitable for displaying character CHARACTER at buffer or string position POS. OBJECT is the string object, or nil for buffer. This macro is only meaningful for multibyte character CHAR. */ - -#define FACE_FOR_CHAR(F, FACE, CHAR, POS, OBJECT) \ - face_for_char ((F), (FACE), (CHAR), (POS), (OBJECT)) - -#else /* not HAVE_WINDOW_SYSTEM */ - -#define FACE_SUITABLE_FOR_ASCII_CHAR_P(FACE, CHAR) \ - ((void) (FACE), (void) (CHAR), true) -#define FACE_FOR_CHAR(F, FACE, CHAR, POS, OBJECT) \ - ((void) (F), (void) (FACE), (void) (CHAR), (void) (POS), \ - (void) (OBJECT), (FACE)->id) - -#endif /* not HAVE_WINDOW_SYSTEM */ +INLINE int +FACE_FOR_CHAR (struct frame *f, struct face *face, int character, + ptrdiff_t pos, Lisp_Object object) +{ +#ifdef HAVE_WINDOW_SYSTEM + return face_for_char (f, face, character, pos, object); +#else + return face->id; +#endif +} /* Return true if G contains a valid character code. */ INLINE bool diff --git a/src/frame.c b/src/frame.c index 9048452..aa06a38 100644 --- a/src/frame.c +++ b/src/frame.c @@ -106,39 +106,32 @@ decode_any_frame (register Lisp_Object frame) } #ifdef HAVE_WINDOW_SYSTEM - bool -window_system_available (struct frame *f) +display_available (void) { - return f ? FRAME_WINDOW_P (f) || FRAME_MSDOS_P (f) : x_display_list != NULL; + return x_display_list != NULL; } +#endif struct frame * decode_window_system_frame (Lisp_Object frame) { struct frame *f = decode_live_frame (frame); - - if (!window_system_available (f)) - error ("Window system frame should be used"); + check_window_system (f); +#ifdef HAVE_WINDOW_SYSTEM return f; +#endif } -#else /* not HAVE_WINDOW_SYSTEM */ - -_Noreturn void -decode_window_system_frame (Lisp_Object frame) -{ - error ("Window system is not in use"); -} - -_Noreturn -#endif /* not HAVE_WINDOW_SYSTEM */ void check_window_system (struct frame *f) { - if (!window_system_available (f)) - error (f ? "Window system frame should be used" - : "Window system is not in use or not initialized"); +#ifdef HAVE_WINDOW_SYSTEM + if (window_system_available (f)) + return; +#endif + error (f ? "Window system frame should be used" + : "Window system is not in use or not initialized"); } /* Return the value of frame parameter PROP in frame FRAME. */ @@ -2137,20 +2130,18 @@ DEFUN ("iconify-frame", Ficonify_frame, Siconify_frame, If omitted, FRAME defaults to the currently selected frame. */) (Lisp_Object frame) { -#ifdef HAVE_WINDOW_SYSTEM struct frame *f = decode_live_frame (frame); -#else - (void) decode_live_frame (frame); -#endif /* Don't allow minibuf_window to remain on an iconified frame. */ check_minibuf_window (frame, EQ (minibuf_window, selected_window)); /* I think this should be done with a hook. */ -#ifdef HAVE_WINDOW_SYSTEM if (FRAME_WINDOW_P (f)) + { +#ifdef HAVE_WINDOW_SYSTEM x_iconify_frame (f); #endif + } /* Make menu bar update for the Buffers and Frames menus. */ windows_or_buffers_changed = 17; @@ -3013,20 +3004,18 @@ or bottom edge of the outer frame of FRAME relative to the right or bottom edge of FRAME's display. */) (Lisp_Object frame, Lisp_Object x, Lisp_Object y) { -#ifdef HAVE_WINDOW_SYSTEM - register struct frame *f = decode_live_frame (frame); -#else - (void) decode_live_frame (frame); -#endif + struct frame *f = decode_live_frame (frame); CHECK_TYPE_RANGED_INTEGER (int, x); CHECK_TYPE_RANGED_INTEGER (int, y); /* I think this should be done with a hook. */ -#ifdef HAVE_WINDOW_SYSTEM if (FRAME_WINDOW_P (f)) - x_set_offset (f, XINT (x), XINT (y), 1); + { +#ifdef HAVE_WINDOW_SYSTEM + x_set_offset (f, XINT (x), XINT (y), 1); #endif + } return Qt; } @@ -3755,8 +3744,8 @@ x_set_left_fringe (struct frame *f, Lisp_Object new_value, Lisp_Object old_value if (new_width != old_width) { - FRAME_LEFT_FRINGE_WIDTH (f) = new_width; - FRAME_FRINGE_COLS (f) /* Round up. */ + f->left_fringe_width = new_width; + f->fringe_cols /* Round up. */ = (new_width + FRAME_RIGHT_FRINGE_WIDTH (f) + unit - 1) / unit; if (FRAME_X_WINDOW (f) != 0) @@ -3779,8 +3768,8 @@ x_set_right_fringe (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu if (new_width != old_width) { - FRAME_RIGHT_FRINGE_WIDTH (f) = new_width; - FRAME_FRINGE_COLS (f) /* Round up. */ + f->right_fringe_width = new_width; + f->fringe_cols /* Round up. */ = (new_width + FRAME_LEFT_FRINGE_WIDTH (f) + unit - 1) / unit; if (FRAME_X_WINDOW (f) != 0) @@ -3809,13 +3798,11 @@ void x_set_right_divider_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) { int old = FRAME_RIGHT_DIVIDER_WIDTH (f); - CHECK_TYPE_RANGED_INTEGER (int, arg); - FRAME_RIGHT_DIVIDER_WIDTH (f) = XINT (arg); - if (FRAME_RIGHT_DIVIDER_WIDTH (f) < 0) - FRAME_RIGHT_DIVIDER_WIDTH (f) = 0; - if (FRAME_RIGHT_DIVIDER_WIDTH (f) != old) + int new = max (0, XINT (arg)); + if (new != old) { + f->right_divider_width = new; adjust_frame_size (f, -1, -1, 4, 0, Qright_divider_width); adjust_frame_glyphs (f); SET_FRAME_GARBAGED (f); @@ -3827,13 +3814,11 @@ void x_set_bottom_divider_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval) { int old = FRAME_BOTTOM_DIVIDER_WIDTH (f); - CHECK_TYPE_RANGED_INTEGER (int, arg); - FRAME_BOTTOM_DIVIDER_WIDTH (f) = XINT (arg); - if (FRAME_BOTTOM_DIVIDER_WIDTH (f) < 0) - FRAME_BOTTOM_DIVIDER_WIDTH (f) = 0; - if (FRAME_BOTTOM_DIVIDER_WIDTH (f) != old) + int new = max (0, XINT (arg)); + if (new != old) { + f->bottom_divider_width = new; adjust_frame_size (f, -1, -1, 4, 0, Qbottom_divider_width); adjust_frame_glyphs (f); SET_FRAME_GARBAGED (f); diff --git a/src/frame.h b/src/frame.h index 4ee0a74..5e3ee68 100644 --- a/src/frame.h +++ b/src/frame.h @@ -1102,23 +1102,37 @@ extern Lisp_Object selected_frame; extern int frame_default_tool_bar_height; #endif +#ifdef HAVE_WINDOW_SYSTEM +# define WINDOW_SYSTEM_RETURN +#else +# define WINDOW_SYSTEM_RETURN _Noreturn +#endif + +extern WINDOW_SYSTEM_RETURN struct frame * + decode_window_system_frame (Lisp_Object); extern struct frame *decode_live_frame (Lisp_Object); extern struct frame *decode_any_frame (Lisp_Object); extern struct frame *make_initial_frame (void); extern struct frame *make_frame (bool); #ifdef HAVE_WINDOW_SYSTEM -extern void check_window_system (struct frame *); -extern struct frame *decode_window_system_frame (Lisp_Object); extern struct frame *make_minibuffer_frame (void); extern struct frame *make_frame_without_minibuffer (Lisp_Object, struct kboard *, Lisp_Object); -extern bool window_system_available (struct frame *); -#else /* not HAVE_WINDOW_SYSTEM */ -extern _Noreturn void check_window_system (struct frame *); -extern _Noreturn void decode_window_system_frame (Lisp_Object); -#define window_system_available(f) ((void) (f), false) -#endif /* HAVE_WINDOW_SYSTEM */ +extern bool display_available (void); +#endif + +INLINE bool +window_system_available (struct frame *f) +{ +#ifdef HAVE_WINDOW_SYSTEM + return f ? FRAME_WINDOW_P (f) || FRAME_MSDOS_P (f) : display_available (); +#else + return false; +#endif +} + +extern WINDOW_SYSTEM_RETURN void check_window_system (struct frame *); extern void frame_make_pointer_invisible (struct frame *); extern void frame_make_pointer_visible (struct frame *); extern Lisp_Object delete_frame (Lisp_Object, Lisp_Object); @@ -1152,46 +1166,68 @@ extern Lisp_Object Vframe_list; This value currently equals the average width of the default font of F. */ #define FRAME_COLUMN_WIDTH(F) ((F)->column_width) -/* Pixel width of areas used to display truncation marks, continuation - marks, overlay arrows. This is 0 for terminal frames. */ +/* Get a frame's window system dimension. If no window system, this is 0. */ +INLINE int +frame_dimension (int x) +{ #ifdef HAVE_WINDOW_SYSTEM + return x; +#else + return 0; +#endif +} /* Total width of fringes reserved for drawing truncation bitmaps, continuation bitmaps and alike. The width is in canonical char units of the frame. This must currently be the case because window sizes aren't pixel values. If it weren't the case, we wouldn't be able to split windows horizontally nicely. */ -#define FRAME_FRINGE_COLS(F) ((F)->fringe_cols) +INLINE int +FRAME_FRINGE_COLS (struct frame *f) +{ + return frame_dimension (f->fringe_cols); +} /* Pixel-width of the left and right fringe. */ -#define FRAME_LEFT_FRINGE_WIDTH(F) ((F)->left_fringe_width) -#define FRAME_RIGHT_FRINGE_WIDTH(F) ((F)->right_fringe_width) +INLINE int +FRAME_LEFT_FRINGE_WIDTH (struct frame *f) +{ + return frame_dimension (f->left_fringe_width); +} +INLINE int +FRAME_RIGHT_FRINGE_WIDTH (struct frame *f) +{ + return frame_dimension (f->right_fringe_width); +} /* Total width of fringes in pixels. */ -#define FRAME_TOTAL_FRINGE_WIDTH(F) \ - (FRAME_LEFT_FRINGE_WIDTH (F) + FRAME_RIGHT_FRINGE_WIDTH (F)) +INLINE int +FRAME_TOTAL_FRINGE_WIDTH (struct frame *f) +{ + return FRAME_LEFT_FRINGE_WIDTH (f) + FRAME_RIGHT_FRINGE_WIDTH (f); +} /* Pixel-width of internal border lines */ -#define FRAME_INTERNAL_BORDER_WIDTH(F) ((F)->internal_border_width) +INLINE int +FRAME_INTERNAL_BORDER_WIDTH (struct frame *f) +{ + return frame_dimension (f->internal_border_width); +} /* Pixel-size of window border lines */ -#define FRAME_RIGHT_DIVIDER_WIDTH(F) ((F)->right_divider_width) -#define FRAME_BOTTOM_DIVIDER_WIDTH(F) ((F)->bottom_divider_width) - -#else /* not HAVE_WINDOW_SYSTEM */ - -#define FRAME_FRINGE_COLS(F) ((void) (F), 0) -#define FRAME_TOTAL_FRINGE_WIDTH(F) ((void) (F), 0) -#define FRAME_LEFT_FRINGE_WIDTH(F) ((void) (F), 0) -#define FRAME_RIGHT_FRINGE_WIDTH(F) ((void) (F), 0) -#define FRAME_INTERNAL_BORDER_WIDTH(F) ((void) (F), 0) -#define FRAME_RIGHT_DIVIDER_WIDTH(F) ((void) (F), 0) -#define FRAME_BOTTOM_DIVIDER_WIDTH(F) ((void) (F), 0) - -#endif /* not HAVE_WINDOW_SYSTEM */ +INLINE int +FRAME_RIGHT_DIVIDER_WIDTH (struct frame *f) +{ + return frame_dimension (f->right_divider_width); +} +INLINE int +FRAME_BOTTOM_DIVIDER_WIDTH (struct frame *f) +{ + return frame_dimension (f->bottom_divider_width); +} \f /*********************************************************************** Conversion between canonical units and pixels diff --git a/src/gnutls.c b/src/gnutls.c index 8ee066f..7f05ac4 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -383,13 +383,6 @@ gnutls_log_function2 (int level, const char *string, const char *extra) message ("gnutls.c: [%d] %s %s", level, string, extra); } -/* Log a message and an integer. */ -static void -gnutls_log_function2i (int level, const char *string, int extra) -{ - message ("gnutls.c: [%d] %s %d", level, string, extra); -} - int gnutls_try_handshake (struct Lisp_Process *proc) { diff --git a/src/gnutls.h b/src/gnutls.h index 47e11f2..41769a4 100644 --- a/src/gnutls.h +++ b/src/gnutls.h @@ -71,7 +71,7 @@ typedef enum #define GNUTLS_LOG2i(level, max, string, extra) \ do { \ if ((level) <= (max)) \ - gnutls_log_function2i (level, "(Emacs) " string, extra); \ + message ("gnutls.c: [%d] %s %d", level, string, extra); \ } while (false) extern ptrdiff_t diff --git a/src/image.c b/src/image.c index 0df415c..572557d 100644 --- a/src/image.c +++ b/src/image.c @@ -80,9 +80,6 @@ typedef struct w32_bitmap_record Bitmap_Record; #define PIX_MASK_DRAW 1 #define x_defined_color w32_defined_color -#if 0 /* unused */ -#define DefaultDepthOfScreen(screen) (one_w32_display_info.n_cbits) -#endif #endif /* HAVE_NTGUI */ diff --git a/src/lisp.h b/src/lisp.h index 6a8f829..e0eb52a 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4634,10 +4634,8 @@ enum STR's value is not necessarily copied. The resulting Lisp string should not be modified or made visible to user code. */ -/* Avoid initializing NAME to prevent "jump-misses-init" compiler - warnings. */ #define AUTO_STRING_WITH_LEN(name, str, len) \ - Lisp_Object name; name = \ + Lisp_Object name = \ (USE_STACK_STRING \ ? (make_lisp_ptr \ ((&(union Aligned_String) \ diff --git a/src/vm-limit.c b/src/vm-limit.c index 7eeca3c..58e7729 100644 --- a/src/vm-limit.c +++ b/src/vm-limit.c @@ -54,10 +54,10 @@ char data_start[1] = { 1 }; #ifdef HAVE_MALLOC_H # include <malloc.h> #endif -#ifndef __MALLOC_HOOK_VOLATILE -# define __MALLOC_HOOK_VOLATILE volatile -#endif #ifndef HAVE_MALLOC_H +# ifndef __MALLOC_HOOK_VOLATILE +# define __MALLOC_HOOK_VOLATILE volatile +# endif extern void *(*__morecore) (ptrdiff_t); extern void (*__MALLOC_HOOK_VOLATILE __after_morecore_hook) (void); #endif diff --git a/src/xdisp.c b/src/xdisp.c index da0e84f..9df73f2 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -5040,11 +5040,9 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object, return 1; #ifdef HAVE_WINDOW_SYSTEM - int fringe_bitmap; - value = XCAR (XCDR (spec)); - if (!SYMBOLP (value) - || !(fringe_bitmap = lookup_fringe_bitmap (value))) + int fringe_bitmap = SYMBOLP (value) ? lookup_fringe_bitmap (value) : 0; + if (! fringe_bitmap) /* If we return here, POSITION has been advanced across the text with this property. */ { @@ -21691,9 +21689,7 @@ Value is the new character position of point. */) int pt_x, target_x, pixel_width, pt_vpos; bool at_eol_p; bool overshoot_expected = false; -#ifdef HAVE_WINDOW_SYSTEM bool target_is_eol_p = false; -#endif /* Setup the arena. */ SET_TEXT_POS (pt, PT, PT_BYTE); @@ -21808,9 +21804,7 @@ Value is the new character position of point. */) { move_it_by_lines (&it, -1); target_x = it.last_visible_x - !FRAME_WINDOW_P (it.f); -#ifdef HAVE_WINDOW_SYSTEM target_is_eol_p = true; -#endif /* Under word-wrap, we don't know the x coordinate of the last character displayed on the previous line, which immediately precedes the wrap point. To find @@ -21851,7 +21845,6 @@ Value is the new character position of point. */) } /* Move to the target X coordinate. */ -#ifdef HAVE_WINDOW_SYSTEM /* On GUI frames, as we don't know the X coordinate of the character to the left of point, moving point to the left requires walking, one grapheme cluster at a time, until we @@ -21908,9 +21901,7 @@ Value is the new character position of point. */) new_pos.bytepos = CHAR_TO_BYTE (new_pos.charpos); it.current.pos = new_pos; } - else -#endif - if (it.current_x != target_x) + else if (it.current_x != target_x) move_it_in_display_line_to (&it, ZV, target_x, MOVE_TO_POS | MOVE_TO_X); /* If we ended up in a display string that covers point, move to @@ -28602,11 +28593,7 @@ static void show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw) { struct window *w = XWINDOW (hlinfo->mouse_face_window); -#ifdef HAVE_WINDOW_SYSTEM struct frame *f = XFRAME (WINDOW_FRAME (w)); -#else - (void) XFRAME (WINDOW_FRAME (w)); -#endif if (/* If window is in the process of being destroyed, don't bother to do anything. */ @@ -28617,9 +28604,7 @@ show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw) anymore. This can happen when a window is split. */ && hlinfo->mouse_face_end_row < w->current_matrix->nrows) { -#ifdef HAVE_WINDOW_SYSTEM bool phys_cursor_on_p = w->phys_cursor_on_p; -#endif struct glyph_row *row, *first, *last; first = MATRIX_ROW (w->current_matrix, hlinfo->mouse_face_beg_row); @@ -28695,12 +28680,12 @@ show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw) } } -#ifdef HAVE_WINDOW_SYSTEM /* When we've written over the cursor, arrange for it to be displayed again. */ if (FRAME_WINDOW_P (f) && phys_cursor_on_p && !w->phys_cursor_on_p) { +#ifdef HAVE_WINDOW_SYSTEM int hpos = w->phys_cursor.hpos; /* When the window is hscrolled, cursor hpos can legitimately be @@ -28715,8 +28700,8 @@ show_mouse_face (Mouse_HLInfo *hlinfo, enum draw_glyphs_face draw) display_and_set_cursor (w, true, hpos, w->phys_cursor.vpos, w->phys_cursor.x, w->phys_cursor.y); unblock_input (); - } #endif /* HAVE_WINDOW_SYSTEM */ + } } #ifdef HAVE_WINDOW_SYSTEM @@ -29656,12 +29641,17 @@ Returns the alist element for the first matching AREA in MAP. */) clip_to_bounds (INT_MIN, XINT (x), INT_MAX), clip_to_bounds (INT_MIN, XINT (y), INT_MAX)); } +#endif /* HAVE_WINDOW_SYSTEM */ /* Display frame CURSOR, optionally using shape defined by POINTER. */ static void define_frame_cursor1 (struct frame *f, Cursor cursor, Lisp_Object pointer) { +#ifdef HAVE_WINDOW_SYSTEM + if (!FRAME_WINDOW_P (f)) + return; + /* Do not change cursor shape while dragging mouse. */ if (EQ (do_mouse_tracking, Qdragging)) return; @@ -29678,10 +29668,10 @@ define_frame_cursor1 (struct frame *f, Cursor cursor, Lisp_Object pointer) cursor = FRAME_X_OUTPUT (f)->horizontal_drag_cursor; else if (EQ (pointer, intern ("nhdrag"))) cursor = FRAME_X_OUTPUT (f)->vertical_drag_cursor; -#ifdef HAVE_X_WINDOWS +# ifdef HAVE_X_WINDOWS else if (EQ (pointer, intern ("vdrag"))) cursor = FRAME_DISPLAY_INFO (f)->vertical_scroll_bar_cursor; -#endif +# endif else if (EQ (pointer, intern ("hourglass"))) cursor = FRAME_X_OUTPUT (f)->hourglass_cursor; else if (EQ (pointer, Qmodeline)) @@ -29692,10 +29682,9 @@ define_frame_cursor1 (struct frame *f, Cursor cursor, Lisp_Object pointer) if (cursor != No_Cursor) FRAME_RIF (f)->define_frame_cursor (f, cursor); +#endif } -#endif /* HAVE_WINDOW_SYSTEM */ - /* Take proper action when mouse has moved to the mode or header line or marginal area AREA of window W, x-position X and y-position Y. X is relative to the start of the text display area of W, so the @@ -29711,9 +29700,9 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y, Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f); #ifdef HAVE_WINDOW_SYSTEM Display_Info *dpyinfo; +#endif Cursor cursor = No_Cursor; Lisp_Object pointer = Qnil; -#endif int dx, dy, width, height; ptrdiff_t charpos; Lisp_Object string, object = Qnil; @@ -29964,12 +29953,8 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y, && hlinfo->mouse_face_beg_row == vpos ) return; -#ifdef HAVE_WINDOW_SYSTEM if (clear_mouse_face (hlinfo)) cursor = No_Cursor; -#else - (void) clear_mouse_face (hlinfo); -#endif if (!row->reversed_p) { @@ -30003,10 +29988,8 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y, show_mouse_face (hlinfo, DRAW_MOUSE_FACE); mouse_face_shown = true; -#ifdef HAVE_WINDOW_SYSTEM if (NILP (pointer)) pointer = Qhand; -#endif } } @@ -30015,10 +29998,7 @@ note_mode_line_or_margin_highlight (Lisp_Object window, int x, int y, if ((area == ON_MODE_LINE || area == ON_HEADER_LINE) && !mouse_face_shown) clear_mouse_face (hlinfo); -#ifdef HAVE_WINDOW_SYSTEM - if (FRAME_WINDOW_P (f)) - define_frame_cursor1 (f, cursor, pointer); -#endif + define_frame_cursor1 (f, cursor, pointer); } @@ -30037,10 +30017,8 @@ note_mouse_highlight (struct frame *f, int x, int y) enum window_part part = ON_NOTHING; Lisp_Object window; struct window *w; -#ifdef HAVE_WINDOW_SYSTEM Cursor cursor = No_Cursor; Lisp_Object pointer = Qnil; /* Takes precedence over cursor! */ -#endif struct buffer *b; /* When a menu is active, don't highlight because this looks odd. */ @@ -30223,19 +30201,17 @@ note_mouse_highlight (struct frame *f, int x, int y) && glyph->type == STRETCH_GLYPH && glyph->avoid_cursor_p)) { -#ifndef HAVE_WINDOW_SYSTEM - (void) clear_mouse_face (hlinfo); -#else /* HAVE_WINDOW_SYSTEM */ if (clear_mouse_face (hlinfo)) cursor = No_Cursor; if (FRAME_WINDOW_P (f) && NILP (pointer)) { +#ifdef HAVE_WINDOW_SYSTEM if (area != TEXT_AREA) cursor = FRAME_X_OUTPUT (f)->nontext_cursor; else pointer = Vvoid_text_area_pointer; +#endif } -#endif /* HAVE_WINDOW_SYSTEM */ goto set_cursor; } @@ -30280,10 +30256,8 @@ note_mouse_highlight (struct frame *f, int x, int y) same_region = coords_in_mouse_face_p (w, hpos, vpos); -#ifdef HAVE_WINDOW_SYSTEM if (same_region) cursor = No_Cursor; -#endif /* Check mouse-face highlighting. */ if (! same_region @@ -30310,12 +30284,8 @@ note_mouse_highlight (struct frame *f, int x, int y) hlinfo->mouse_face_overlay = overlay; /* Clear the display of the old active region, if any. */ -#ifdef HAVE_WINDOW_SYSTEM if (clear_mouse_face (hlinfo)) cursor = No_Cursor; -#else - (void) clear_mouse_face (hlinfo); -#endif /* If no overlay applies, get a text property. */ if (NILP (overlay)) @@ -30346,9 +30316,7 @@ note_mouse_highlight (struct frame *f, int x, int y) = face_at_string_position (w, object, pos, 0, &ignore, glyph->face_id, true); show_mouse_face (hlinfo, DRAW_MOUSE_FACE); -#ifdef HAVE_WINDOW_SYSTEM cursor = No_Cursor; -#endif } else { @@ -30432,9 +30400,7 @@ note_mouse_highlight (struct frame *f, int x, int y) : XFASTINT (after), before_string, after_string, disp_string); -#ifdef HAVE_WINDOW_SYSTEM cursor = No_Cursor; -#endif } } } @@ -30557,15 +30523,7 @@ note_mouse_highlight (struct frame *f, int x, int y) } set_cursor: - -#ifdef HAVE_WINDOW_SYSTEM - if (FRAME_WINDOW_P (f)) - define_frame_cursor1 (f, cursor, pointer); -#else - /* This is here to prevent a compiler error, about "label at end of - compound statement". */ - return; -#endif + define_frame_cursor1 (f, cursor, pointer); } diff --git a/src/xfaces.c b/src/xfaces.c index faf28fc..97a5ae0 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -5287,11 +5287,11 @@ realize_default_face (struct frame *f) eassert (lface_fully_specified_p (XVECTOR (lface)->contents)); check_lface (lface); memcpy (attrs, XVECTOR (lface)->contents, sizeof attrs); - -#ifndef HAVE_X_WINDOWS - (void) realize_face (c, attrs, DEFAULT_FACE_ID); -#else /* HAVE_X_WINDOWS */ struct face *face = realize_face (c, attrs, DEFAULT_FACE_ID); + +#ifndef HAVE_WINDOW_SYSTEM + (void) face; +#else if (FRAME_X_P (f) && face->font != FRAME_FONT (f)) { /* This can happen when making a frame on a display that does @@ -5305,7 +5305,7 @@ realize_default_face (struct frame *f) font. */ x_set_font (f, LFACE_FONT (lface), Qnil); } -#endif /* HAVE_X_WINDOWS */ +#endif return true; } @@ -6093,7 +6093,7 @@ face_at_string_position (struct window *w, Lisp_Object string, if we don't have fonts, so we can stop here if not working on a window-system frame. */ || !FRAME_WINDOW_P (f) - || FACE_SUITABLE_FOR_ASCII_CHAR_P (base_face, 0))) + || FACE_SUITABLE_FOR_ASCII_CHAR_P (base_face))) return base_face->id; /* Begin with attributes from the base face. */ diff --git a/src/xfns.c b/src/xfns.c index 35e2a23..1120c33 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -1434,7 +1434,7 @@ x_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldva if (border != FRAME_INTERNAL_BORDER_WIDTH (f)) { - FRAME_INTERNAL_BORDER_WIDTH (f) = border; + f->internal_border_width = border; #ifdef USE_X_TOOLKIT if (FRAME_X_OUTPUT (f)->edit_widget) -- 2.5.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-22 1:12 ` Paul Eggert @ 2016-06-22 6:10 ` martin rudalics 2016-06-23 7:15 ` Paul Eggert 2016-06-22 14:04 ` Andy Moreton 2016-06-22 14:06 ` Ken Brown 2 siblings, 1 reply; 14+ messages in thread From: martin rudalics @ 2016-06-22 6:10 UTC (permalink / raw) To: Paul Eggert, Ken Brown; +Cc: 23771 > +/* Get a frame's window system dimension. If no window system, this is 0. */ > > +INLINE int > +frame_dimension (int x) frame_dimension sounds misleading to me. Couldn't you call it something like frame_area_dimension or frame_subarea_dimension instead? Thanks, martin ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-22 6:10 ` martin rudalics @ 2016-06-23 7:15 ` Paul Eggert 0 siblings, 0 replies; 14+ messages in thread From: Paul Eggert @ 2016-06-23 7:15 UTC (permalink / raw) To: martin rudalics, Ken Brown; +Cc: 23771 On 06/22/2016 08:10 AM, martin rudalics wrote: > frame_dimension sounds misleading to me. Couldn't you call it something > like frame_area_dimension or frame_subarea_dimension instead? Sure, please feel free to change the name, I didn't think much about it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-22 1:12 ` Paul Eggert 2016-06-22 6:10 ` martin rudalics @ 2016-06-22 14:04 ` Andy Moreton 2016-06-22 14:10 ` Ken Brown 2016-06-22 14:06 ` Ken Brown 2 siblings, 1 reply; 14+ messages in thread From: Andy Moreton @ 2016-06-22 14:04 UTC (permalink / raw) To: 23771 On Wed 22 Jun 2016, Paul Eggert wrote: > Thanks for all the work in improving static checking for Cygwin builds. I just > now checked the attached patch into master, which I hope improves on it. I > tested it on Fedora (both with and without window systems). > > I had one problem with the recent changes, in that they suppressed all > warnings about jumps over AUTO_STRING calls. That's pretty drastic, as the > warnings are typically useful, so the attached patch reverts that. Can you let > me know which call needs the warning suppressed in the Cygwin-specific code in > the new master? I can suggest something which disables the warning just for > that call. Your patch changed FRAME_INTERNAL_BORDER_WIDTH() from a macro to an inline function, which breaks the mingw64 build: ../../src/w32fns.c: In function 'x_set_internal_border_width': ../../src/w32fns.c:1661:39: error: lvalue required as left operand of assignment FRAME_INTERNAL_BORDER_WIDTH (f) = border; ^ With FRAME_INTERNAL_BORDER_WIDTH() as a macro, it builds successfully. AndyM ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-22 14:04 ` Andy Moreton @ 2016-06-22 14:10 ` Ken Brown 0 siblings, 0 replies; 14+ messages in thread From: Ken Brown @ 2016-06-22 14:10 UTC (permalink / raw) To: Andy Moreton, 23771 On 6/22/2016 10:04 AM, Andy Moreton wrote: > Your patch changed FRAME_INTERNAL_BORDER_WIDTH() from a macro to an > inline function, which breaks the mingw64 build: > > ../../src/w32fns.c: In function 'x_set_internal_border_width': > ../../src/w32fns.c:1661:39: error: lvalue required as left operand of assignment > FRAME_INTERNAL_BORDER_WIDTH (f) = border; I fixed this in commit 81fc9a7. Ken ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-22 1:12 ` Paul Eggert 2016-06-22 6:10 ` martin rudalics 2016-06-22 14:04 ` Andy Moreton @ 2016-06-22 14:06 ` Ken Brown 2 siblings, 0 replies; 14+ messages in thread From: Ken Brown @ 2016-06-22 14:06 UTC (permalink / raw) To: Paul Eggert; +Cc: 23771 On 6/21/2016 9:12 PM, Paul Eggert wrote: > Thanks for all the work in improving static checking for Cygwin builds. > I just now checked the attached patch into master, which I hope improves > on it. I tested it on Fedora (both with and without window systems). Thanks. That's a big improvement. > I had one problem with the recent changes, in that they suppressed all > warnings about jumps over AUTO_STRING calls. That's pretty drastic, as > the warnings are typically useful, so the attached patch reverts that. > Can you let me know which call needs the warning suppressed in the > Cygwin-specific code in the new master? I can suggest something which > disables the warning just for that call. It's in line 7057 of w32fns.c. I see that you fixed a similar warning in xfns.c in commit e0400b7 by moving the AUTO_STRING call. I've just done the same thing in commit bbc58fe. Ken ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-15 2:04 bug#23771: Eliminating compiler warnings Ken Brown 2016-06-15 14:44 ` Eli Zaretskii @ 2016-06-15 20:24 ` Richard Stallman 2016-06-16 1:41 ` Ken Brown 1 sibling, 1 reply; 14+ messages in thread From: Richard Stallman @ 2016-06-15 20:24 UTC (permalink / raw) To: Ken Brown; +Cc: 23771 [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > I would like to get rid of all compiler warnings in the Cygwin builds > (X11, w32, and nox). There are currently none in the X11 build, but > there are a lot in the other builds. It could be that the best way to get rid of some of these warnings is to delete some -W option that causes GCC to issue them. In the case of _Noreturn, isn't there a simpler approach, defining a macro that expands into either _Noreturn or nothing? Or defining _Noreturn as a no-op when there is no other definition? > +#ifdef HAVE_WINDOW_SYSTEM > int fringe_bitmap; > +#endif Surely it is easier to put in some code that will unconditionally use that variable. But maybe just delete the -W option that causes these warnings. To clutter up the code to placate a finicky compiler is a change for the worse. If the compiler is so finicky only because a special option asked it to be, why be massochistic? -- Dr Richard Stallman President, Free Software Foundation (gnu.org, fsf.org) Internet Hall-of-Famer (internethalloffame.org) Skype: No way! See stallman.org/skype.html. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#23771: Eliminating compiler warnings 2016-06-15 20:24 ` Richard Stallman @ 2016-06-16 1:41 ` Ken Brown 0 siblings, 0 replies; 14+ messages in thread From: Ken Brown @ 2016-06-16 1:41 UTC (permalink / raw) To: rms; +Cc: 23771 On 6/15/2016 4:24 PM, Richard Stallman wrote: > To clutter up the code to placate a finicky compiler is a change > for the worse. If the compiler is so finicky only because a special > option asked it to be, why be massochistic? I'm just using the warning options that are enabled by default for a build from a git checkout of the master branch. In general, I think the warnings are useful. But I agree that I went too far in trying to get rid of them all. I'm now thinking that I should just live with the ones that can't be fixed without cluttering the code. See my reply to Eli. Ken ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-06-23 7:15 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-15 2:04 bug#23771: Eliminating compiler warnings Ken Brown 2016-06-15 14:44 ` Eli Zaretskii 2016-06-16 1:38 ` Ken Brown 2016-06-16 15:14 ` Eli Zaretskii 2016-06-16 15:50 ` Paul Eggert 2016-06-21 3:11 ` Ken Brown 2016-06-22 1:12 ` Paul Eggert 2016-06-22 6:10 ` martin rudalics 2016-06-23 7:15 ` Paul Eggert 2016-06-22 14:04 ` Andy Moreton 2016-06-22 14:10 ` Ken Brown 2016-06-22 14:06 ` Ken Brown 2016-06-15 20:24 ` Richard Stallman 2016-06-16 1:41 ` Ken Brown
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).