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

ffrankies added a comment. In D66564#2260836 , @aaron.ballman wrote: > I've committed on your behalf in 156b127945a8c923d141e608b7380427da024376 > . Thank > you for the new check! No

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

aaron.ballman closed this revision. aaron.ballman added a comment. I've committed on your behalf in 156b127945a8c923d141e608b7380427da024376 . Thank you for the new check! CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

ffrankies added a comment. In D66564#2257635 , @aaron.ballman wrote: > In D66564#2256482 , @ffrankies wrote: > >> In D66564#2256424 , @Eugene.Zelenko >> wrote: >> >>> In D6

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

ffrankies updated this revision to Diff 290396. ffrankies added a comment. Rebased, changed import in `StructPackAlignCheck.h` from `../ClangTidy.h` to `../ClangTidyCheck.h` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 Files: clang-tools-e

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

aaron.ballman added a comment. In D66564#2256482 , @ffrankies wrote: > In D66564#2256424 , @Eugene.Zelenko > wrote: > >> In D66564#2256423 , @ffrankies >> wrote: >> >>> @Eu

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

ffrankies added a comment. In D66564#2256424 , @Eugene.Zelenko wrote: > In D66564#2256423 , @ffrankies wrote: > >> @Eugene.Zelenko I don't have commit access to the repository, could you >> please commit this chec

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

Eugene.Zelenko added a comment. In D66564#2256423 , @ffrankies wrote: > @Eugene.Zelenko I don't have commit access to the repository, could you > please commit this check on our behalf? Sorry, I don't have it either. @aaron.ballman or @njames93 could do

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

ffrankies added a comment. @Eugene.Zelenko I don't have commit access to the repository, could you please commit this check on our behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 ___ cfe-commi

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

ffrankies added a comment. @njames93 Thanks for the clarification! Your suggestion worked, but then I realized that I was working off of an improperly worded comment, which I've corrected. I looked through the AST Matcher reference, and didn't find anything that would trigger on a templated str

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

ffrankies updated this revision to Diff 283106. ffrankies added a comment. Clarified a comment: we don't want the warnings to trigger on templated struct //declarations//, not instantiations. We don't want it to trigger on any instantiations. Added a test case that checks if warnings are trigge

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

njames93 added a comment. In 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, b

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

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(isTe

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor nit with the matcher, this LGTM! Comment at: clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp:50-53 + // Do not trigger on templ

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

Eugene.Zelenko added a comment. In D66564#2049664 , @ffrankies wrote: > @Eugene.Zelenko Just checking in, is there anything I missed regarding what > we need to do for these checks? It'll be good idea to ping reviewers. Repository: rG LLVM Github Mo

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

ffrankies added a comment. @Eugene.Zelenko Just checking in, is there anything I missed regarding what we need to do for these checks? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564

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

ffrankies updated this revision to Diff 254277. ffrankies added a comment. Addressed comments by @aaron.ballman You're right, we don't want this to trigger on template instantiations and structs declared in system headers. - Check no longer triggers on template instantiations - Check no longer

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

aaron.ballman added a comment. In D66564#1895980 , @Eugene.Zelenko wrote: > In D66564#1895916 , @aaron.ballman > wrote: > > > Are you aware of plans that you or others have for adding additional checks > > to the

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

Eugene.Zelenko added a comment. In 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. CHANGES SINCE

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

aaron.ballman added a comment. Are you aware of plans that you or others have for adding additional checks to the new `altera` module? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 ___ cfe-commits

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

ffrankies updated this revision to Diff 246871. ffrankies marked 5 inline comments as done. ffrankies added a comment. Addressed comments by @aaron.ballman - Removed commented-out code and irrelevant FIXMEs - Added Fix-Its to insert/amend the aligned and packed struct attributes as needed. CHA

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

aaron.ballman added inline comments. Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:66-68 + // FIXME: Express this as CharUnit rather than a hardcoded 8-bits (Rshift3)i + // After computing the minimum size in bits, check for an existing alignment + // flag. --

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

ffrankies updated this revision to Diff 230828. ffrankies edited the summary of this revision. ffrankies added a comment. Implemented changes requested by @aaron.ballman - Moved the check from the "performance" module into the new "altera" module - Split the diagnostic message into a warning and