[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Parse/ParseTentative.cpp:2130
+  ParsedAttributesWithRange attrs(AttrFactory);
+  ParseGNUAttributes(attrs, nullptr, nullptr);
+  if (attrs.size() <= 0) {
----------------
It's not correct in general to call arbitrary parsing actions in a tentatively-parsed region; they might annotate or otherwise mess with the token stream in undesirable ways, or produce diagnostics, etc. If we keep this tentative parsing formulation, you'll need to instead recognize the `__attribute__` token then manually skip its attribute list.


================
Comment at: clang/lib/Parse/ParseTentative.cpp:2146
+bool Parser::isNullStmtWithAttributes() {
+  RevertingTentativeParsingAction PA(*this);
+  return TryParseNullStmtWithAttributes() == TPResult::True;
----------------
xbolva00 wrote:
> Is this “cheap” in terms of compile time?
No; this is not a reasonable thing to do for every block-scope statement or declaration that we parse.

We should do the same thing that we do for all other kinds of attribute: parse them unconditionally then reject them in the contexts where they should not be permitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838