[PATCH] Fix "warning: fallthrough annotation does not directly precede switch label" in lambdas.

[Aaron Ballman]

On Mon, Jun 23, 2014 at 4:13 PM, Alexander Kornienko <alexfh at google.com> wrote:
>
>
>
> On Mon, Jun 23, 2014 at 7:04 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> > Index: lib/Sema/AnalysisBasedWarnings.cpp
>> > ===================================================================
>> > --- lib/Sema/AnalysisBasedWarnings.cpp
>> > +++ lib/Sema/AnalysisBasedWarnings.cpp
>> > @@ -1025,6 +1025,9 @@
>> >      // methods separately.
>> >      bool TraverseDecl(Decl *D) { return true; }
>> >
>> > +    // We analyze lambda bodies separately. Skip them here.
>> > +    bool TraverseLambdaBody(LambdaExpr *LE) { return true; }
>> > +
>> >    private:
>> >
>> >      static const AttributedStmt *asFallThroughAttr(const Stmt *S) {
>> > @@ -1084,6 +1087,9 @@
>> >    if (PerFunction && FM.getFallthroughStmts().empty())
>> >      return;
>> >
>> > +  llvm::errs() << "DiagnoseSwitchLabelsFallthrough: \n";
>> > +  AC.getBody()->dumpColor();
>>
>> This looks like debugging code that should likely go away.
>
>
> Sure, removed.
>
>>
>>
>> > +
>> >    CFG *Cfg = AC.getCFG();
>> >
>> >    if (!Cfg)
>> > Index: test/SemaCXX/switch-implicit-fallthrough.cpp
>> > ===================================================================
>> > --- test/SemaCXX/switch-implicit-fallthrough.cpp
>> > +++ test/SemaCXX/switch-implicit-fallthrough.cpp
>> > @@ -229,25 +229,6 @@
>> >    return n;
>> >  }
>> >
>> > -int fallthrough_targets(int n) {
>> > -  [[clang::fallthrough]]; // expected-error{{fallthrough annotation is
>> > outside switch statement}}
>> > -
>> > -  [[clang::fallthrough]]  // expected-error{{fallthrough attribute is
>> > only allowed on empty statements}}
>> > -  switch (n) {
>> > -    case 121:
>> > -      n += 400;
>> > -      [[clang::fallthrough]]; // no warning here, correct target
>> > -    case 123:
>> > -      [[clang::fallthrough]]  // expected-error{{fallthrough attribute
>> > is only allowed on empty statements}}
>> > -      n += 800;
>> > -      break;
>> > -    [[clang::fallthrough]]    // expected-error{{fallthrough attribute
>> > is only allowed on empty statements}} expected-note{{did you forget ';'?}}
>> > -    case 125:
>> > -      n += 1600;
>> > -  }
>> > -  return n;
>> > -}
>>
>> Why did this test move within the file?
>
>
> Clang seems to stop producing warnings after the function containing errors,
> so the test with errors should be the last one (or maybe even in a separate
> file).

Hmmm, yeah, just for analysis-based warnings. What's strange is that
those are issued on a per-method basis, but any error (not upgraded
from a warning) in the TU causes the analysis warnings to not be fired
for the rest of the TU. That doesn't seem like particularly helpful
behavior, but it also has nothing to do with your patch.

New patch LGTM!

~Aaron