[PATCH] D111568: [clang] Omit most AttributedStatements from the CFG

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Analysis/CFG.cpp:2407
+  return hasSpecificAttr<FallThroughAttr>(A->getAttrs()) &&
+         isa<NullStmt>(A->getSubStmt());
+}
----------------
thakis wrote:
> hans wrote:
> > Can fallthrough statements ever have children? If not, should it be an assert instead of a check here?
> Good question. Attr.td says:
> 
> ```
>   // The attribute only applies to a NullStmt, but we have special fix-it
>   // behavior if applied to a case label.
>   let Subjects = SubjectList<[NullStmt, SwitchCase], ErrorDiag,
>                              "empty statements">;
> ```
> 
> Which I suppose triggers for this:
> 
> ```
>   switch (argc) {
>     [[fallthrough]] case 4:
>       break;
>   }
> ```
> 
> ```
> foo.cc:6:7: error: 'fallthrough' attribute is only allowed on empty statements
>     [[fallthrough]] case 4:
>       ^             ~~~~
> foo.cc:6:20: note: did you forget ';'?
>     [[fallthrough]] case 4:
> ```
> 
> But that doesn't seem to make it into the AST, according to -dump-ast. So I suppose it could be an assert as well. Want me to change this?
> 
> 
Yes, I think an assert would make sense, otherwise the reader has to think about what would the code be doing for an AttributedStmt with non-null substmt.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111568/new/

https://reviews.llvm.org/D111568