Re: Implement -Wimplicit-fallthrough (version 7)

[Marek Polacek <polacek@redhat.com>]

On Mon, Aug 29, 2016 at 04:02:33PM -0400, Jason Merrill wrote:
> On Mon, Aug 29, 2016 at 7:58 AM, Marek Polacek <polacek@redhat.com> wrote:
> > --- gcc/gcc/cp/parser.c
> > +++ gcc/gcc/cp/parser.c
> > @@ -5135,6 +5135,31 @@ cp_parser_primary_expression (cp_parser *parser,
> >         case RID_AT_SELECTOR:
> >           return cp_parser_objc_expression (parser);
> >
> > +       case RID_ATTRIBUTE:
> > +         {
> > +           /* This might be __attribute__((fallthrough));.  */
> > +           tree attr = cp_parser_gnu_attributes_opt (parser);
> > +           if (attr != NULL_TREE
> > +               && is_attribute_p ("fallthrough", get_attribute_name (attr)))
> > +             {
> > +               tree fn = build_call_expr_internal_loc (token->location,
> > +                                                       IFN_FALLTHROUGH,
> > +                                                       void_type_node, 0);
> > +               return cp_expr (fn, token->location);
> > +             }
> > +           else if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
> > +             {
> > +               cp_parser_error (parser, "only attribute %<fallthrough%> "
> > +                                "can be used");
> 
> Let's say "can be applied to a null statement".
 
Fixed.

> > @@ -10585,15 +10610,26 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
> >         }
> >        /* Look for an expression-statement instead.  */
> >        statement = cp_parser_expression_statement (parser, in_statement_expr);
> > +
> > +      /* Handle [[fallthrough]];.  */
> > +      if (std_attrs != NULL_TREE
> > +         && is_attribute_p ("fallthrough", get_attribute_name (std_attrs))
> > +         && statement == NULL_TREE)
> > +       {
> > +         tree fn = build_call_expr_internal_loc (statement_location,
> > +                                                 IFN_FALLTHROUGH,
> > +                                                 void_type_node, 0);
> 
> Let's use the 'statement' variable rather than a new variable fn so
> that we set the call's location below.  Let's also warn here about
> [[fallthrough]] on non-null expression statements.

Done.  But I couldn't figure out a testcase where the warning would trigger, so
not sure how important that is.

> > +         finish_expr_stmt (fn);
> 
> Let's clear std_attrs here...
> 
> > +       }
> 
> >    /* Set the line number for the statement.  */
> >    if (statement && STATEMENT_CODE_P (TREE_CODE (statement)))
> >      SET_EXPR_LOCATION (statement, statement_location);
> >
> > -  /* Note that for now, we don't do anything with c++11 statements
> > -     parsed at this level.  */
> > -  if (std_attrs != NULL_TREE)
> > +  /* Allow "[[fallthrough]];", but warn otherwise.  */
> > +  if (std_attrs != NULL_TREE
> > +      && !is_attribute_p ("fallthrough", get_attribute_name (std_attrs)))
> 
> ...so we don't need to check for fallthrough here.
 
Ok, done.

> > @@ -24114,6 +24164,16 @@ cp_parser_std_attribute (cp_parser *parser)
> >                      " use %<gnu::deprecated%>");
> >           TREE_PURPOSE (TREE_PURPOSE (attribute)) = get_identifier ("gnu");
> >         }
> > +      /* C++17 fallthrough attribute is equivalent to GNU's.  */
> > +      else if (cxx_dialect >= cxx11
> 
> There's no point checking this, here or for attribute deprecated; if
> we're in this function, we must be in C++11 or above.  Let's drop the
> check in both places.

Makes sense, fixed.

Thanks for looking into this.  I'll post another version once it passes
testing.

	Marek