aaron.ballman added inline comments.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5223
+/// matches the declaration of foo and bar.
+AST_MATCHER(VarDecl, isConstinit) { return Node.hasAttr<ConstInitAttr>(); }
+
----------------
This isn't quite correct -- there are two forms of `ConstInitAttr`:
```
constinit int i = 0; // We want to match this one
[[clang::require_constant_initialization]] int j = 0; // We don't want to match
this one
```
We typically want AST matchers to model the AST and this one does not... but, I
think it is okay. `constinit` was based on the implementation experience we got
with `[[clang::require_constant_initialization]]` and so we continue to model
it as an attribute rather than a bit on the AST node. Having this matcher helps
to distinguish the cases (otherwise we could just use the `hasAttr()` matcher
instead).
I think the logic here should be:
```
if (const auto *CIA = Node.getAttr<ConstInitAttr>())
return CIA->isConstinit();
return false;
```
and you should add a test case + documentation to show that we don't match the
attribute form.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117846/new/
https://reviews.llvm.org/D117846
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
