martong added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:26
+
+ if (NewExpr->getNumPlacementArgs() > 0)
+ return;
----------------
Perhaps we should add in the docs that placement new is not supported. Or add a
fixme here.
Anyway, I feel this code simply could work with placement new as well. What is
the reason you disabled it for placement new?
================
Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:41
+ unsigned SpecifiedAlignment = D->getMaxAlignment();
+ unsigned DefaultAlignment = Context.getTargetInfo().getCharAlign();
+ if (!SpecifiedAlignment)
----------------
This might not be what we want... `getCharAlign()` theoretically could return
even with `1`, I think, though it would be a very strange architecture.
Perhaps we should use `getSuitableAlign()` instead?
Also, I think we should call the variable as `FundamentalAlignment`.
Fundamental and default alignment can differ, I guess.
================
Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:49
+ if (HasDefaultOperatorNew && OverAligned)
+ diag(NewExpr->getBeginLoc(), "using default 'operator new' with
over-aligned type %0 may result in undefined behavior")
+ << D;
----------------
I think it would be useful to add the value of the fundamental alignment to the
diagnostics too.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:107
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment)
<cert-oop54-cpp>
- clang-analyzer-core.CallAndMessage
- clang-analyzer-core.DivideZero
+ clang-analyzer-core.CallAndMessage (redirects to
https://clang.llvm.org/docs/analyzer/checkers)
<clang-analyzer-core.CallAndMessage>
+ clang-analyzer-core.DivideZero (redirects to
https://clang.llvm.org/docs/analyzer/checkers) <clang-analyzer-core.DivideZero>
----------------
Why do we have these changes? Seems to be unrelated.
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
