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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
