This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: diagnostic: fix PR c++/19756


On 01/12/06, Dirk Mueller <dmuell@gmx.net> wrote:

Hi,

Hi. First, I should mention I cannot approve your patch. Actually I am a newbie here.

I don't quite understand why the implementation in the C frontend is so overly
complicated, while the simple patch (see below) works as well.

Overly complicated? Let's see the code in c-typeck.c (c_finish_if_stmt)


 /* Diagnose an ambiguous else if if-then-else is nested inside if-then.  */
 if (warn_parentheses && nested_if && else_block == NULL)
   {
     tree inner_if = then_block;

     /* We know from the grammar productions that there is an IF nested
        within THEN_BLOCK.  Due to labels and c99 conditional declarations,
        it might not be exactly THEN_BLOCK, but should be the last
        non-container statement within.  */
     while (1)
       switch (TREE_CODE (inner_if))
         {
         case COND_EXPR:
           goto found;
         case BIND_EXPR:
           inner_if = BIND_EXPR_BODY (inner_if);
           break;
         case STATEMENT_LIST:
           inner_if = expr_last (then_block);
           break;
         case TRY_FINALLY_EXPR:
         case TRY_CATCH_EXPR:
           inner_if = TREE_OPERAND (inner_if, 0);
           break;
         default:
           gcc_unreachable ();
         }
   found:

     if (COND_EXPR_ELSE (inner_if))
        warning (OPT_Wparentheses,
                 "%Hsuggest explicit braces to avoid ambiguous %<else%>",
                 &if_locus);
   }

It seems to me that this code is handling some C99 constructions that
are not handled in your patch (presumably because they don't exist in
C++).

Other than that, I think the differences are pretty minor. For
example. COND_EXPR_ELSE is practically the same as ELSE_CLAUSE.
Perhaps, it could be advantageous to unify the code and move it to
c-common.c


              }
+           else
+           {
+             tree inner_then = THEN_CLAUSE (statement);
+

I think you can move this initialisation...


+             if (warn_parentheses && no_then_braces)
+               {

... here and actually remove one indentation level.


+                 if (TREE_CODE (inner_then) == STATEMENT_LIST
+                     && STATEMENT_LIST_TAIL (inner_then))
+                   inner_then = STATEMENT_LIST_TAIL (inner_then)->stmt;

I guess there is no practical difference but, why not use inner_then = expr_last (inner_then) here? The latter seems more abstract to me. (Actually, just a few lines above the definition of STATEMENT_LIST_TAIL it says "Use the interface in tree-iterator.h to access this node").

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]