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

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

In D66564#1895980 <https://reviews.llvm.org/D66564#1895980>, @Eugene.Zelenko wrote:

> In D66564#1895916 <https://reviews.llvm.org/D66564#1895916>, @aaron.ballman wrote:
>
> > Are you aware of plans that you or others have for adding additional checks to the new `altera` module?
>
>
> There are several patches for `altera` module already.


Excellent, thank you for the confirmation!



================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:23
+void StructPackAlignCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(recordDecl(isStruct(), isDefinition()).bind("struct"),
+                     this);
----------------
I don't think we want this check to trigger on template instantiations because the packing and alignment requirements may be different there for each instantiation type. e.g.,
```
template <typename Ty, typename Uy>
struct S {
  Ty One;
  int Two;
  Uy Three;
};
```


================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:87
+  if (NeedsPacking && !IsPacked) {
+    diag(Struct->getLocation(),
+         "accessing fields in struct %0 is inefficient due to padding; only "
----------------
Should this be diagnosing structures declared in system headers? My intuition is that we don't want to diagnose those because they're outside of the user's control. This can be handled when registering the matchers for the check, if we want to ignore those (`unless(isExpansionInSystemHeader())`).


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

https://reviews.llvm.org/D66564