[clang] [Attributes] Support Attributes being declared as only supporting late parsing when passing an experimental feature flag (PR #88596)

Dan Liew via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 18 10:37:58 PDT 2024


================
@@ -91,11 +91,24 @@ static StringRef normalizeAttrName(StringRef Name) {
 
 /// isAttributeLateParsed - Return true if the attribute has arguments that
 /// require late parsing.
-static bool isAttributeLateParsed(const IdentifierInfo &II) {
+bool Parser::isAttributeLateParsed(const IdentifierInfo &II) {
----------------
delcypher wrote:

@rapidsna Good catch. Thinking about this some more I think the semantics you proposed creates a problem here.

The semantics require (in psuedo code) that the function do this:

```
bool Parser::isAttributeLateParsed(const IdentifierInfo &II) {
  if (<attribute is experimental late parsed>) {
    if (getLangOpts().ExperimentalLateParseAttributes)
      return true;
    } else {
      // Fall back to Standard late parsed behavior which means we need to return true here.
      return true;
  }

  return <attribute is standard late parsed>;
}
```

Which simplifies to

```
bool Parser::isAttributeLateParsed(const IdentifierInfo &II) {
  return <attribute is experimental late parsed> ||   <attribute is standard late parsed>
```

So basically

* `getLangOpts().ExperimentalLateParseAttributes` basically does nothing.
* I'm not convinced this function is the right place for us to control late parsing. Even before this patch this function would return true (regardless of input language) for attributes that were marked as `LateParsed`. 
E.g. for `guarded_by` this would return true I think.

So I think we either need to

* Use the semantics I had before but use better names (my preference)
* Use the semantics you proposed but figure out where we need to conditionalize on `getLangOpts().ExperimentalLateParseAttributes`.

Let me know what you think.



https://github.com/llvm/llvm-project/pull/88596


More information about the cfe-commits mailing list