aaron.ballman added inline comments.
Herald added a subscriber: hiraditya.
================
Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:68-70
+ // MEM
+ CheckFactories.registerCheck<DefaultOperatorNewAlignmentCheck>(
+ "cert-mem57-cpp");
----------------
The `MEM` section should come before `MSC`.
================
Comment at:
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:24
+void DefaultOperatorNewAlignmentCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ // Get the found 'new' expression.
----------------
Didn't we decide the check should not diagnose in C++17 or later?
================
Comment at:
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:29-30
+ // Skip placement new: This is a user-defined allocation.
+ if (NewExpr->getNumPlacementArgs() > 0)
+ return;
+ QualType T = NewExpr->getAllocatedType();
----------------
I think this should be hoisted into a local AST matcher rather than be part of
the `check()` call. Something like `unless(isPlacementNew())` when registering
the check.
================
Comment at:
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:47
+ // The user-specified alignment (in bits).
+ const unsigned SpecifiedAlignment = D->getMaxAlignment();
+ // Double-check if no alignment was specified.
----------------
Please drop top-level `const` on anything that's not a pointer or a reference
(that's not a style we typically follow).
================
Comment at:
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:59
+ const unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+ if (HasDefaultOperatorNew && OverAligned /*&& !NewExpr->passAlignment()*/)
+ diag(NewExpr->getBeginLoc(),
----------------
Commented-out code?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67545/new/
https://reviews.llvm.org/D67545
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
