Re: [PATCH GCC]Add 'force-dwarf-lexical-blocks' command line option

["Joseph S. Myers" <joseph@codesourcery.com>]

On Sun, 1 Jun 2014, Herman, Andrei wrote:

>+  /* The -fforce-dwarf-lexical-blocks option is only relevant when debug
>+     info is in DWARF4 format */
>+  if (flag_force_dwarf_blocks) {

Watch coding style: the opening '{' always goes on the next line.

>+fforce-dwarf-lexical-blocks
>+C C++ Var(flag_force_dwarf_blocks)
>+Force generation of lexical blocks in dwarf output

I don't see a good reason for this not to be supported for ObjC and ObjC++ 
as well.  Say DWARF, not dwarf.

>+@item -fforce-dwarf-lexical-blocks
>+Produce debug information (a DW_TAG_lexical_block) for every function
>+body, loop body, switch body, case statement, if-then and if-else statement,
>+even if the body is a single statement.  Likewise, a lexical block will be
>+emitted for the first label of a statement.  This block ends at the end of the
>+current lexical scope, or when a break, continue, goto or return statement is
>+encountered at the same lexical scope level.  This option is usefull for
>+coverage tools that utilize the dwarf debug information.
>+This option only applies to C/C++ code and is available when using DWARF
>+Version 4 or higher.

Use @code{} markup for keywords (if, else, break, continue, goto, return).  
"useful" not "usefull".  "DWARF" not "dwarf".

>+/* Create a block_loc struct for a statement list created on behalf of
>+   flag_force_dwarf_blocks.  We use this for label or forced c99 scopes.  */
>+
>+void
>+push_block_info (tree block, location_t loc, bool is_label)
>+{
>+  if (TREE_CODE(block) != STATEMENT_LIST)

Watch coding style: space before '(' in function and macro calls (and 
similar calls such as sizeof) (many places in this patch, not just this 
one).

>+tree
>+pop_block_info (location_t &loc)

It's not documented in codingconventions.html, but I think it's preferred 
to avoid returning values through reference arguments (see e.g. 
<https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00198.html>).

>+{
>+  block_loc  tl = NULL;

Excess space between "block_loc" and "tl".

>@@ -4679,7 +4712,7 @@ c_parser_compound_statement_nostart (c_parser *parser)
>    expressions being rejected later.  */
> 
> static void
>-c_parser_label (c_parser *parser)
>+c_parser_label (c_parser *parser, bool prev_label)

You're adding a new argument - you need to update the comment above this 
function to explain the semantics of this argument.

In general, make sure that new functions have comments above them that 
explain the semantics of the arguments (by name) and any return value.

>+/* If current scope is a label scope, pop it from block info stack
>+   and close it's compound statement.  */

"its" not "it's".

-- 
Joseph S. Myers
joseph@codesourcery.com