[PATCH] D66564: [clang-tidy] new altera struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies added a comment.

Posted this as an inline comment, copying here:

I tried to move this into the matcher like this:

  Finder->addMatcher(recordDecl(isStruct(), isDefinition(),
                                unless(isExpansionInSystemHeader()),
                                unless(isTemplateInstantiation()))
                         .bind("struct"),
                     this);

but got an error asking for a TemplateSpecializationKind parameter to the isTemplateInstantiation function. Did some more digging and according to the docs, this matcher is only available for the CXXRecordDecl class, not the generic RecordDecl.

Is there a way around this? And if not, do we ignore it? Lastly, do I need to do anything else since this check has been accepted? There is a "Close Revision" action in the comments, but I'm not sure if that will push the changes or not.



================
Comment at: clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp:50-53
+  // Do not trigger on template instantiations because the packing and
+  // alignment requirements are unknown.
+  if (Struct->isTemplated())
+    return;
----------------
aaron.ballman wrote:
> I think this should be hoisted into the AST matcher using `unless(isTemplateInstantiation())`.
I tried to move this into the matcher like this:

```
  Finder->addMatcher(recordDecl(isStruct(), isDefinition(),
                                unless(isExpansionInSystemHeader()),
                                unless(isTemplateInstantiation()))
                         .bind("struct"),
                     this);
```

but got an error asking for a `TemplateSpecializationKind` to the `isTemplateInstantiation` function. Did some more digging and according to the docs, this matcher is only available for the CXXRecordDecl class, not the generic RecordDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66564