* [RFA 2/3] Add missing ATTRIBUTE_NORETURNs
2016-09-27 18:16 [RFA 0/3] Fix some -Wimplicit-fallthrough warnings Tom Tromey
@ 2016-09-27 18:16 ` Tom Tromey
2016-09-28 7:28 ` Yao Qi
2016-09-27 18:16 ` [RFA 1/3] Fix "fall through" comments Tom Tromey
2016-09-27 18:52 ` [RFA 3/3] Fix "obvious" fall-through warnings Tom Tromey
2 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2016-09-27 18:16 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch adds a couple of missing ATTRIBUTE_NORETURNs. This lets
-Wimplicit-fallthrough recognize that a given case does not fall
through.
2016-09-27 Tom Tromey <tom@tromey.com>
* dwarf2loc.c (unimplemented): Add ATTRIBUTE_NORETURN.
* completer.h (throw_max_completions_reached_error): Add
ATTRIBUTE_NORETURN.
---
gdb/ChangeLog | 6 ++++++
gdb/completer.h | 3 ++-
gdb/dwarf2loc.c | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fdd43ab..83b051f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2016-09-27 Tom Tromey <tom@tromey.com>
+ * dwarf2loc.c (unimplemented): Add ATTRIBUTE_NORETURN.
+ * completer.h (throw_max_completions_reached_error): Add
+ ATTRIBUTE_NORETURN.
+
+2016-09-27 Tom Tromey <tom@tromey.com>
+
* xcoffread.c (scan_xcoff_symtab): Move comment later.
* symfile.c (section_is_mapped): Fix fall-through comment.
* stabsread.c (define_symbol, read_member_functions): Fix
diff --git a/gdb/completer.h b/gdb/completer.h
index 24bfee9..ff01f86 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -179,6 +179,7 @@ extern enum maybe_add_completion_enum
/* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR. */
-extern void throw_max_completions_reached_error (void);
+extern void throw_max_completions_reached_error (void)
+ ATTRIBUTE_NORETURN;
#endif /* defined (COMPLETER_H) */
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index f9f3216..a29c9ed 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2917,7 +2917,7 @@ dwarf2_loc_desc_get_symbol_read_needs (const gdb_byte *data, size_t size,
/* A helper function that throws an unimplemented error mentioning a
given DWARF operator. */
-static void
+static void ATTRIBUTE_NORETURN
unimplemented (unsigned int op)
{
const char *name = get_DW_OP_name (op);
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 1/3] Fix "fall through" comments
2016-09-27 18:16 [RFA 0/3] Fix some -Wimplicit-fallthrough warnings Tom Tromey
2016-09-27 18:16 ` [RFA 2/3] Add missing ATTRIBUTE_NORETURNs Tom Tromey
@ 2016-09-27 18:16 ` Tom Tromey
2016-09-27 21:48 ` Yao Qi
2016-09-27 18:52 ` [RFA 3/3] Fix "obvious" fall-through warnings Tom Tromey
2 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2016-09-27 18:16 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch updates existing "fall through" comments so that they can
be recognized by gcc's -Wimplicit-fallthrough comment-parsing
heuristic.
2016-09-27 Tom Tromey <tom@tromey.com>
* xcoffread.c (scan_xcoff_symtab): Move comment later.
* symfile.c (section_is_mapped): Fix fall-through comment.
* stabsread.c (define_symbol, read_member_functions): Fix
fall-through comment.
* s390-linux-tdep.c (s390_process_record): Fix fall-through
comment.
* remote.c (remote_wait_as): Fix fall-through comment.
* p-exp.y (yylex): Fix fall-through comment.
* nat/x86-dregs.c (x86_length_and_rw_bits): Fix fall-through
comment.
* msp430-tdep.c (msp430_gdbarch_init): Fix fall-through comment.
* mdebugread.c (parse_partial_symbols): Fix fall-through comment.
* jv-exp.y (yylex): Fix fall-through comment.
* go-exp.y (lex_one_token): Fix fall-through comment.
* gdbtypes.c (get_discrete_bounds, rank_one_type): Fix
fall-through comment.
* f-exp.y (yylex): Fix fall-through comment.
* dwarf2read.c (process_die): Fix fall-through comment.
* dbxread.c (process_one_symbol): Fix fall-through comment.
* d-exp.y (lex_one_token): Fix fall-through comment.
* cp-name-parser.y (yylex): Fix fall-through comment.
* coffread.c (coff_symtab_read): Fix fall-through comment.
* c-exp.y (lex_one_token): Fix fall-through comment.
* arm-tdep.c (arm_decode_miscellaneous): Fix fall-through
comment.
* arch/arm.c (arm_instruction_changes_pc): Fix fall-through
comment.
---
gdb/ChangeLog | 30 ++++++++++++++++++++++++++++++
gdb/arch/arm.c | 3 ++-
gdb/arm-tdep.c | 1 +
gdb/c-exp.y | 2 +-
gdb/coffread.c | 3 ++-
gdb/cp-name-parser.y | 2 +-
gdb/d-exp.y | 2 +-
gdb/dbxread.c | 4 ++--
gdb/dwarf2read.c | 3 ++-
gdb/f-exp.y | 2 +-
gdb/gdbtypes.c | 6 +++---
gdb/go-exp.y | 2 +-
gdb/jv-exp.y | 2 +-
gdb/mdebugread.c | 3 ++-
gdb/msp430-tdep.c | 2 +-
gdb/nat/x86-dregs.c | 2 +-
gdb/p-exp.y | 2 +-
gdb/remote.c | 2 +-
gdb/s390-linux-tdep.c | 5 +++--
gdb/stabsread.c | 5 +++--
gdb/symfile.c | 2 +-
gdb/xcoffread.c | 2 +-
22 files changed, 62 insertions(+), 25 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4810f1b..fdd43ab 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,33 @@
+2016-09-27 Tom Tromey <tom@tromey.com>
+
+ * xcoffread.c (scan_xcoff_symtab): Move comment later.
+ * symfile.c (section_is_mapped): Fix fall-through comment.
+ * stabsread.c (define_symbol, read_member_functions): Fix
+ fall-through comment.
+ * s390-linux-tdep.c (s390_process_record): Fix fall-through
+ comment.
+ * remote.c (remote_wait_as): Fix fall-through comment.
+ * p-exp.y (yylex): Fix fall-through comment.
+ * nat/x86-dregs.c (x86_length_and_rw_bits): Fix fall-through
+ comment.
+ * msp430-tdep.c (msp430_gdbarch_init): Fix fall-through comment.
+ * mdebugread.c (parse_partial_symbols): Fix fall-through comment.
+ * jv-exp.y (yylex): Fix fall-through comment.
+ * go-exp.y (lex_one_token): Fix fall-through comment.
+ * gdbtypes.c (get_discrete_bounds, rank_one_type): Fix
+ fall-through comment.
+ * f-exp.y (yylex): Fix fall-through comment.
+ * dwarf2read.c (process_die): Fix fall-through comment.
+ * dbxread.c (process_one_symbol): Fix fall-through comment.
+ * d-exp.y (lex_one_token): Fix fall-through comment.
+ * cp-name-parser.y (yylex): Fix fall-through comment.
+ * coffread.c (coff_symtab_read): Fix fall-through comment.
+ * c-exp.y (lex_one_token): Fix fall-through comment.
+ * arm-tdep.c (arm_decode_miscellaneous): Fix fall-through
+ comment.
+ * arch/arm.c (arm_instruction_changes_pc): Fix fall-through
+ comment.
+
2016-09-27 Fredrik Hederstierna <fredrik.hederstierna@verisure.com>
* arm-tdep.c (arm_m_addr_is_magic): New function.
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index b6770fe..03ed7c4 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -153,7 +153,8 @@ arm_instruction_changes_pc (uint32_t this_instr)
modify PC. */
return 0;
}
- /* Data processing instruction. Fall through. */
+ /* Data processing instruction. */
+ /* Fall through. */
case 0x1:
if (bits (this_instr, 12, 15) == 15)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index a07d93b..3fc5e40 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -6655,6 +6655,7 @@ arm_decode_miscellaneous (struct gdbarch *gdbarch, uint32_t insn,
else if (op == 0x3)
/* Not really supported. */
return arm_copy_unmodified (gdbarch, insn, "smc", dsc);
+ /* Fall through. */
default:
return arm_copy_undef (gdbarch, insn, dsc);
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 29f672f..73589d4 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2527,7 +2527,7 @@ lex_one_token (struct parser_state *par_state, int *is_quoted_name)
last_was_structop = 1;
goto symbol; /* Nope, must be a symbol. */
}
- /* FALL THRU into number case. */
+ /* FALL THRU. */
case '0':
case '1':
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 830deb5..49d6803 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -968,7 +968,8 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
/* At least on a 3b1, gcc generates swbeg and string labels
that look like this. Ignore them. */
break;
- /* Fall in for static symbols that don't start with '.' */
+ /* For static symbols that don't start with '.'... */
+ /* Fall through. */
case C_THUMBEXT:
case C_THUMBEXTFUNC:
case C_EXT:
diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index c6a5c34..e15b0f6 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -1673,7 +1673,7 @@ yylex (void)
lexptr++;
return '-';
}
- /* FALL THRU into number case. */
+ /* FALL THRU. */
try_number:
case '0':
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index 426f9b3..a3287e6 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -1123,7 +1123,7 @@ lex_one_token (struct parser_state *par_state)
last_was_structop = 1;
goto symbol; /* Nope, must be a symbol. */
}
- /* FALL THRU into number case. */
+ /* FALL THRU. */
case '0':
case '1':
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index d5a9587..bef28db 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -3069,9 +3069,9 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
unknown_symtype_complaint (hex_string (type));
/* FALLTHROUGH */
- /* The following symbol types don't need the address field
- relocated, since it is either unused, or is absolute. */
define_a_symbol:
+ /* These symbol types don't need the address field relocated,
+ since it is either unused, or is absolute. */
case N_GSYM: /* Global variable. */
case N_NSYMS: /* Number of symbols (Ultrix). */
case N_NOMAP: /* No map? (Ultrix). */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d0f6e71..42b5704 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8339,7 +8339,8 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
cu->processing_has_namespace_info = 1;
if (read_namespace_alias (die, cu))
break;
- /* The declaration is not a global namespace alias: fall through. */
+ /* The declaration is not a global namespace alias. */
+ /* fall through. */
case DW_TAG_imported_module:
cu->processing_has_namespace_info = 1;
if (die->child != NULL && (die->tag == DW_TAG_imported_declaration
diff --git a/gdb/f-exp.y b/gdb/f-exp.y
index 420f18e..fecfdc9 100644
--- a/gdb/f-exp.y
+++ b/gdb/f-exp.y
@@ -1006,7 +1006,7 @@ yylex (void)
/* Might be a floating point number. */
if (lexptr[1] < '0' || lexptr[1] > '9')
goto symbol; /* Nope, must be a symbol. */
- /* FALL THRU into number case. */
+ /* FALL THRU. */
case '0':
case '1':
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index ad890ca..d2a0f37 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -958,7 +958,7 @@ get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
*highp = -*lowp - 1;
return 0;
}
- /* ... fall through for unsigned ints ... */
+ /* fall through */
case TYPE_CODE_CHAR:
*lowp = 0;
/* This round-about calculation is to avoid shifting by
@@ -3803,7 +3803,7 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
return INTEGER_CONVERSION_BADNESS;
else if (TYPE_LENGTH (arg) < TYPE_LENGTH (parm))
return INTEGER_PROMOTION_BADNESS;
- /* >>> !! else fall through !! <<< */
+ /* fall through */
case TYPE_CODE_CHAR:
/* Deal with signed, unsigned, and plain chars for C++ and
with int cases falling through from previous case. */
@@ -3909,7 +3909,7 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
rank.subrank = distance_to_ancestor (parm, arg, 0);
if (rank.subrank >= 0)
return sum_ranks (BASE_CONVERSION_BADNESS, rank);
- /* else fall through */
+ /* fall through */
default:
return INCOMPATIBLE_TYPE_BADNESS;
}
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 1b0fe5b..2723789 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -1089,7 +1089,7 @@ lex_one_token (struct parser_state *par_state)
last_was_structop = 1;
goto symbol; /* Nope, must be a symbol. */
}
- /* FALL THRU into number case. */
+ /* FALL THRU. */
case '0':
case '1':
diff --git a/gdb/jv-exp.y b/gdb/jv-exp.y
index 79b8127..f9db49a 100644
--- a/gdb/jv-exp.y
+++ b/gdb/jv-exp.y
@@ -921,7 +921,7 @@ yylex (void)
/* Might be a floating point number. */
if (lexptr[1] < '0' || lexptr[1] > '9')
goto symbol; /* Nope, must be a symbol. */
- /* FALL THRU into number case. */
+ /* FALL THRU. */
case '0':
case '1':
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 157ce77..9995659 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -3701,7 +3701,8 @@ parse_partial_symbols (struct objfile *objfile)
break;
default:
unknown_ext_complaint (debug_info->ssext + psh->iss);
- /* Fall through, pretend it's global. */
+ /* Pretend it's global. */
+ /* Fall through. */
case stGlobal:
/* Global common symbols are resolved by the runtime loader,
ignore them. */
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 84189dc..bb03fec 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -923,8 +923,8 @@ msp430_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
code_model = ca_tdep->code_model;
break;
}
- /* Otherwise, fall through... */
}
+ /* Fall through. */
default:
error (_("Unknown msp430 isa"));
break;
diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
index 8403ca7..cb68866 100644
--- a/gdb/nat/x86-dregs.c
+++ b/gdb/nat/x86-dregs.c
@@ -257,7 +257,7 @@ Invalid hardware breakpoint type %d in x86_length_and_rw_bits.\n"),
case 8:
if (TARGET_HAS_DR_LEN_8)
return (DR_LEN_8 | rw);
- /* ELSE FALL THROUGH */
+ /* FALL THROUGH */
default:
internal_error (__FILE__, __LINE__, _("\
Invalid hardware breakpoint length %d in x86_length_and_rw_bits.\n"), len);
diff --git a/gdb/p-exp.y b/gdb/p-exp.y
index fa6b22c..e8612ee 100644
--- a/gdb/p-exp.y
+++ b/gdb/p-exp.y
@@ -1211,7 +1211,7 @@ yylex (void)
goto symbol; /* Nope, must be a symbol. */
}
- /* FALL THRU into number case. */
+ /* FALL THRU. */
case '0':
case '1':
diff --git a/gdb/remote.c b/gdb/remote.c
index ec8b498..d7266db 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6984,7 +6984,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
putpkt (buf);
break;
}
- /* else fallthrough */
+ /* fallthrough */
default:
warning (_("Invalid remote reply: %s"), buf);
break;
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 399084a..0b73b2d 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -5206,7 +5206,7 @@ ex:
return -1;
break;
}
- /* For other instructions, fallthru. */
+ /* fallthru. */
default:
fprintf_unfiltered (gdb_stdlog, "Warning: Unknown KM* function %02x at %s.\n",
(int)tmp, paddress (gdbarch, addr));
@@ -5499,7 +5499,8 @@ ex:
return -1;
break;
}
- /* For KLMD, fallthru. */
+ /* For KLMD... */
+ /* fallthru. */
default:
fprintf_unfiltered (gdb_stdlog, "Warning: Unknown KMAC function %02x at %s.\n",
(int)tmp, paddress (gdbarch, addr));
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index e8ebadd..c3b9531 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -722,7 +722,7 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
/* SunPRO (3.0 at least) static variable encoding. */
if (gdbarch_static_transform_name_p (gdbarch))
goto normal;
- /* ... fall through ... */
+ /* fall through */
default:
complaint (&symfile_complaints, _("Unknown C++ symbol name `%s'"),
@@ -2530,7 +2530,8 @@ read_member_functions (struct field_info *fip, char **pp, struct type *type,
complaint (&symfile_complaints,
_("member function type missing, got '%c'"),
(*pp)[-1]);
- /* Fall through into normal member function. */
+ /* Normal member function. */
+ /* Fall through. */
case '.':
/* normal member function. */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7eb6cdc..e9e8ca7 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3100,7 +3100,7 @@ section_is_mapped (struct obj_section *osect)
if (osect->ovly_mapped == -1)
gdbarch_overlay_update (gdbarch, osect);
}
- /* fall thru to manual case */
+ /* fall thru */
case ovly_on: /* overlay debugging manual */
return osect->ovly_mapped == 1;
}
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 507baf2..70e031c 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -2507,9 +2507,9 @@ scan_xcoff_symtab (struct objfile *objfile)
}
/* FALLTHROUGH */
+ case C_FCN:
/* C_FCN is .bf and .ef symbols. I think it is sufficient
to handle only the C_FUN and C_EXT. */
- case C_FCN:
case C_BSTAT:
case C_ESTAT:
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 0/3] Fix some -Wimplicit-fallthrough warnings
@ 2016-09-27 18:16 Tom Tromey
2016-09-27 18:16 ` [RFA 2/3] Add missing ATTRIBUTE_NORETURNs Tom Tromey
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Tom Tromey @ 2016-09-27 18:16 UTC (permalink / raw)
To: gdb-patches
I enabled -Wimplicit-fallthrough locally and then built gdb.
This patch series fixes all cases that I understood. After this, my
local build (--enable-targets=all) still has 5 remaining warnings (one
in i386-tdep, two in rs6000-tdep, one in linux-record, and one in
stabsread -- all code I don't know very well...).
Built and regtested on x86-64 Fedora 24.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 3/3] Fix "obvious" fall-through warnings
2016-09-27 18:16 [RFA 0/3] Fix some -Wimplicit-fallthrough warnings Tom Tromey
2016-09-27 18:16 ` [RFA 2/3] Add missing ATTRIBUTE_NORETURNs Tom Tromey
2016-09-27 18:16 ` [RFA 1/3] Fix "fall through" comments Tom Tromey
@ 2016-09-27 18:52 ` Tom Tromey
2 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2016-09-27 18:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch fixes the subset of -Wimplicit-fallthrough warnings that I
considered obvious. In most cases it was obvious from context that
falling through was desired; here I added the appropriate comment. In
a couple of cases it seemed clear that a "break" was missing.
2016-09-27 Tom Tromey <tom@tromey.com>
* utils.c (can_dump_core) <LIMIT_CUR>: Add fall-through comment.
* mi/mi-main.c (mi_cmd_trace_frame_collected) <REGISTERS_FORMAT>:
Add missing "break".
* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals) <NO_FRAME_FILTERS>:
Add missing "break".
* eval.c (fetch_subexp_value) <MEMORY_ERROR>: Add fall-through
comment.
* d-valprint.c (d_val_print) <TYPE_CODE_STRUCT>: Add fall-through
comment.
* coffread.c (coff_symtab_read) <C_LABEL>: Add fall-through
comment.
---
gdb/ChangeLog | 14 ++++++++++++++
gdb/coffread.c | 1 +
gdb/d-valprint.c | 1 +
gdb/eval.c | 1 +
gdb/mi/mi-cmd-stack.c | 1 +
gdb/mi/mi-main.c | 1 +
gdb/utils.c | 1 +
7 files changed, 20 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 83b051f..511dedc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
2016-09-27 Tom Tromey <tom@tromey.com>
+ * utils.c (can_dump_core) <LIMIT_CUR>: Add fall-through comment.
+ * mi/mi-main.c (mi_cmd_trace_frame_collected) <REGISTERS_FORMAT>:
+ Add missing "break".
+ * mi/mi-cmd-stack.c (mi_cmd_stack_list_locals) <NO_FRAME_FILTERS>:
+ Add missing "break".
+ * eval.c (fetch_subexp_value) <MEMORY_ERROR>: Add fall-through
+ comment.
+ * d-valprint.c (d_val_print) <TYPE_CODE_STRUCT>: Add fall-through
+ comment.
+ * coffread.c (coff_symtab_read) <C_LABEL>: Add fall-through
+ comment.
+
+2016-09-27 Tom Tromey <tom@tromey.com>
+
* dwarf2loc.c (unimplemented): Add ATTRIBUTE_NORETURN.
* completer.h (throw_max_completions_reached_error): Add
ATTRIBUTE_NORETURN.
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 49d6803..d3ebe97 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -932,6 +932,7 @@ coff_symtab_read (long symtab_offset, unsigned int nsyms,
backtraces, so filter them out (from phdm@macqel.be). */
if (within_function)
break;
+ /* Fall through. */
case C_STAT:
case C_THUMBLABEL:
case C_THUMBSTAT:
diff --git a/gdb/d-valprint.c b/gdb/d-valprint.c
index 620688b..4166a59 100644
--- a/gdb/d-valprint.c
+++ b/gdb/d-valprint.c
@@ -88,6 +88,7 @@ d_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
stream, recurse, val, options);
if (ret == 0)
break;
+ /* Fall through. */
default:
c_val_print (type, valaddr, embedded_offset, address, stream,
recurse, val, options);
diff --git a/gdb/eval.c b/gdb/eval.c
index 00a107c..1f2fb00 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -237,6 +237,7 @@ fetch_subexp_value (struct expression *exp, int *pc, struct value **valp,
case MEMORY_ERROR:
if (!preserve_errors)
break;
+ /* Fall through. */
default:
throw_exception (ex);
break;
diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index 390fc7e..b51650d 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -246,6 +246,7 @@ mi_cmd_stack_list_locals (char *command, char **argv, int argc)
{
case NO_FRAME_FILTERS:
raw_arg = oind;
+ break;
case SKIP_UNAVAILABLE:
skip_unavailable = 1;
break;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 1913157..3b604e5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2815,6 +2815,7 @@ mi_cmd_trace_frame_collected (char *command, char **argv, int argc)
break;
case REGISTERS_FORMAT:
registers_format = oarg[0];
+ break;
case MEMORY_CONTENTS:
memory_contents = 1;
break;
diff --git a/gdb/utils.c b/gdb/utils.c
index 9a83053..9d11780 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -562,6 +562,7 @@ can_dump_core (enum resource_limit_kind limit_kind)
case LIMIT_CUR:
if (rlim.rlim_cur == 0)
return 0;
+ /* Fall through. */
case LIMIT_MAX:
if (rlim.rlim_max == 0)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 1/3] Fix "fall through" comments
2016-09-27 18:16 ` [RFA 1/3] Fix "fall through" comments Tom Tromey
@ 2016-09-27 21:48 ` Yao Qi
2016-09-27 21:53 ` Tom Tromey
0 siblings, 1 reply; 12+ messages in thread
From: Yao Qi @ 2016-09-27 21:48 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Tue, Sep 27, 2016 at 7:16 PM, Tom Tromey <tom@tromey.com> wrote:
> This patch updates existing "fall through" comments so that they can
> be recognized by gcc's -Wimplicit-fallthrough comment-parsing
> heuristic.
>
clang has -Wimplicit-fallthrough as well, but warnings are suppressed in
a different way. Do we need to suppress warnings generated by both gcc
and clang?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 1/3] Fix "fall through" comments
2016-09-27 21:48 ` Yao Qi
@ 2016-09-27 21:53 ` Tom Tromey
2016-09-28 21:51 ` Yao Qi
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2016-09-27 21:53 UTC (permalink / raw)
To: Yao Qi; +Cc: Tom Tromey, gdb-patches
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
Yao> clang has -Wimplicit-fallthrough as well, but warnings are suppressed in
Yao> a different way. Do we need to suppress warnings generated by both gcc
Yao> and clang?
On my machine, clang won't build gdb. From memory there was at least
some issue compiling .c files in c++ mode, but I think there were other
problems as well.
If clang support is desired, then yes, the correct approach is to
replace the fall-through comments with a new attribute.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 2/3] Add missing ATTRIBUTE_NORETURNs
2016-09-27 18:16 ` [RFA 2/3] Add missing ATTRIBUTE_NORETURNs Tom Tromey
@ 2016-09-28 7:28 ` Yao Qi
2016-09-28 17:40 ` Tom Tromey
0 siblings, 1 reply; 12+ messages in thread
From: Yao Qi @ 2016-09-28 7:28 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Tue, Sep 27, 2016 at 7:16 PM, Tom Tromey <tom@tromey.com> wrote:
> /* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR. */
>
> -extern void throw_max_completions_reached_error (void);
> +extern void throw_max_completions_reached_error (void)
> + ATTRIBUTE_NORETURN;
>
Don't we need to also add ATTRIBUTE_NORETURN to the definition of
throw_max_completions_reached_error?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 2/3] Add missing ATTRIBUTE_NORETURNs
2016-09-28 7:28 ` Yao Qi
@ 2016-09-28 17:40 ` Tom Tromey
2016-09-28 17:41 ` Yao Qi
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2016-09-28 17:40 UTC (permalink / raw)
To: Yao Qi; +Cc: Tom Tromey, gdb-patches
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>> -extern void throw_max_completions_reached_error (void);
>> +extern void throw_max_completions_reached_error (void)
>> + ATTRIBUTE_NORETURN;
Yao> Don't we need to also add ATTRIBUTE_NORETURN to the definition of
Yao> throw_max_completions_reached_error?
No, it's sufficient to have an attribute on a prototype, if the
prototype is in scope when the definition is seen. This seems to be the
normal approach in gdb, e.g., see error_no_arg.
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 2/3] Add missing ATTRIBUTE_NORETURNs
2016-09-28 17:40 ` Tom Tromey
@ 2016-09-28 17:41 ` Yao Qi
0 siblings, 0 replies; 12+ messages in thread
From: Yao Qi @ 2016-09-28 17:41 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Wed, Sep 28, 2016 at 4:20 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
>>> -extern void throw_max_completions_reached_error (void);
>>> +extern void throw_max_completions_reached_error (void)
>>> + ATTRIBUTE_NORETURN;
>
> Yao> Don't we need to also add ATTRIBUTE_NORETURN to the definition of
> Yao> throw_max_completions_reached_error?
>
> No, it's sufficient to have an attribute on a prototype, if the
> prototype is in scope when the definition is seen. This seems to be the
> normal approach in gdb, e.g., see error_no_arg.
>
Er... right. Patch is good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 1/3] Fix "fall through" comments
2016-09-27 21:53 ` Tom Tromey
@ 2016-09-28 21:51 ` Yao Qi
2016-09-28 21:58 ` Tom Tromey
0 siblings, 1 reply; 12+ messages in thread
From: Yao Qi @ 2016-09-28 21:51 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Tue, Sep 27, 2016 at 10:10 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Yao> clang has -Wimplicit-fallthrough as well, but warnings are suppressed in
> Yao> a different way. Do we need to suppress warnings generated by both gcc
> Yao> and clang?
>
> On my machine, clang won't build gdb. From memory there was at least
> some issue compiling .c files in c++ mode, but I think there were other
> problems as well.
We can use option '-x c++' to suppress the warnings, but you are right, there
are some other problems, like some warning options are unknown to clang.
>
> If clang support is desired, then yes, the correct approach is to
> replace the fall-through comments with a new attribute.
>
At least, I don't think of a reason that we don't support clang. clang(++)
support is not very good, but we shouldn't make it worse, IMO. I am open
to people's thoughts.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 1/3] Fix "fall through" comments
2016-09-28 21:51 ` Yao Qi
@ 2016-09-28 21:58 ` Tom Tromey
2016-09-30 21:09 ` Yao Qi
0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2016-09-28 21:58 UTC (permalink / raw)
To: Yao Qi; +Cc: Tom Tromey, gdb-patches
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
Tom> On my machine, clang won't build gdb. From memory there was at
Tom> least some issue compiling .c files in c++ mode, but I think there
Tom> were other problems as well.
Yao> We can use option '-x c++' to suppress the warnings, but you are
Yao> right, there are some other problems, like some warning options are
Yao> unknown to clang.
Yeah. I tried it again and there are two issues.
First, clang++ doesn't like the .c extension:
clang-3.8: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
I was able to fix this with:
make CC=clang CXX='clang++ -x c++' CC_LD=clang++
After that I got:
error: unknown warning option '-Wunused-but-set-parameter'; did you mean '-Wunused-parameter'? [-Werror,-Wunknown-warning-option]
error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]
This is probably an oversight in warning.m4, as in config.log I see:
configure:14316: clang++ -c -g -O2 -Wunused-but-set-parameter conftest.cpp >&5
warning: unknown warning option '-Wunused-but-set-parameter'; did you mean '-Wunused-parameter'? [-Wunknown-warning-option]
1 warning generated.
configure:14316: $? = 0
Adding 'WERROR_CFLAGS=' solves this... but at this point it's fine to
keep fall-through comments, because warnings are disabled anyway.
It's worth noting that I see tons of "unused function" warnings from
vec.h. when building this way. There's definitely some work to be done
here if anybody wants to use clang.
In sum my view is that it's fine to go ahead with the comment approach.
If someone wants to fix up the clang build I can help convert the
comments to an attribute. (The issue with doing it up-front is
discovering the spots that would need a change -- gcc won't tell us what
they are.)
thanks,
Tom
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 1/3] Fix "fall through" comments
2016-09-28 21:58 ` Tom Tromey
@ 2016-09-30 21:09 ` Yao Qi
0 siblings, 0 replies; 12+ messages in thread
From: Yao Qi @ 2016-09-30 21:09 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Wed, Sep 28, 2016 at 9:12 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Tom> On my machine, clang won't build gdb. From memory there was at
> Tom> least some issue compiling .c files in c++ mode, but I think there
> Tom> were other problems as well.
>
> Yao> We can use option '-x c++' to suppress the warnings, but you are
> Yao> right, there are some other problems, like some warning options are
> Yao> unknown to clang.
>
> Yeah. I tried it again and there are two issues.
>
> First, clang++ doesn't like the .c extension:
>
> clang-3.8: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
>
> I was able to fix this with:
>
> make CC=clang CXX='clang++ -x c++' CC_LD=clang++
>
> After that I got:
>
> error: unknown warning option '-Wunused-but-set-parameter'; did you mean '-Wunused-parameter'? [-Werror,-Wunknown-warning-option]
> error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]
>
> This is probably an oversight in warning.m4, as in config.log I see:
>
> configure:14316: clang++ -c -g -O2 -Wunused-but-set-parameter conftest.cpp >&5
> warning: unknown warning option '-Wunused-but-set-parameter'; did you mean '-Wunused-parameter'? [-Wunknown-warning-option]
> 1 warning generated.
> configure:14316: $? = 0
>
> Adding 'WERROR_CFLAGS=' solves this... but at this point it's fine to
> keep fall-through comments, because warnings are disabled anyway.
>
> It's worth noting that I see tons of "unused function" warnings from
> vec.h. when building this way. There's definitely some work to be done
> here if anybody wants to use clang.
>
Yes, I see all of them above in my build.
> In sum my view is that it's fine to go ahead with the comment approach.
OK.
> If someone wants to fix up the clang build I can help convert the
> comments to an attribute. (The issue with doing it up-front is
> discovering the spots that would need a change -- gcc won't tell us what
> they are.)
>
Agreed. Your patch is good to me then.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-09-30 19:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-27 18:16 [RFA 0/3] Fix some -Wimplicit-fallthrough warnings Tom Tromey
2016-09-27 18:16 ` [RFA 2/3] Add missing ATTRIBUTE_NORETURNs Tom Tromey
2016-09-28 7:28 ` Yao Qi
2016-09-28 17:40 ` Tom Tromey
2016-09-28 17:41 ` Yao Qi
2016-09-27 18:16 ` [RFA 1/3] Fix "fall through" comments Tom Tromey
2016-09-27 21:48 ` Yao Qi
2016-09-27 21:53 ` Tom Tromey
2016-09-28 21:51 ` Yao Qi
2016-09-28 21:58 ` Tom Tromey
2016-09-30 21:09 ` Yao Qi
2016-09-27 18:52 ` [RFA 3/3] Fix "obvious" fall-through warnings Tom Tromey
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).