[PATCH] D117846: [ASTMatchers] Add `isConstinit` matcher

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 21 12:32:35 PST 2022


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



More information about the cfe-commits mailing list