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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D66564#2185335 <https://reviews.llvm.org/D66564#2185335>, @ffrankies wrote:

> 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.

Once its all ready to land and you're happy it can be committed, If you don't have commit access, ask one of us to commit on your behalf, We would need to know your GitHub name and email mark you as the author of the patch.



================
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;
----------------
ffrankies wrote:
> 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.
For the `TemplateSpecializationKind`, You're getting that error as `isTemplateInstantiation` is a function that's likely been included in, way around that issue is using `ast_matchers::isTemplateInstantiation()`.
For the CXXRecordDecl issue, you can dynCast the recordDecl to a cxxRecordDecl.
The whole code should look something like this
`unless(cxxRecordDecl(ast_matchers::isTemplateInstantiation()))`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66564