[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe8a4c74f1157: [clang-tidy] Added DefaultOperatorNewCheck. (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D67545?vs=226206&id=230019#toc Repository: rG LLVM Github Monorepo

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added a subscriber: lebedev.ri. aaron.ballman added a comment. In D67545#1749579 , @lebedev.ri wrote: > In D67545#1749561 , @balazske wrote: > > > Ping. It should be accepted before I can land it. > >

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. In D67545#1749561 , @balazske wrote: > Ping. It should be accepted before I can land it. @aaron.ballman already accepted. Repository:

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske added a comment. Ping. It should be accepted before I can land it. 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-com

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri added a comment. Land this? 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.llv

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske updated this revision to Diff 226206. balazske added a comment. - Improved check for C++17. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D67545 Files: clang-tools-extra/clang-tidy/cert/CERTTidyModule

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a minor issue. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:30-31 + // Check not applicable in C++17 or newer. + if (getLangOpts().CPlusPlus17 || getLan

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske updated this revision to Diff 225625. balazske added a comment. - Fixes from review comments, added C++ version test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D67545 Files: clang-tools-extra/clan

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. (outstanding unaddressed review notes) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D6754

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

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( +"cert-mem57-cpp"); The `MEM` section should come before `MSC`

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske updated this revision to Diff 222161. balazske added a comment. - Rename to DefaultOperatorNewAlignmentCheck. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D67545 Files: clang-tools-extra/clang-tidy/c

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:69 +// MEM +CheckFactories.registerCheck("cert-mem57-cpp"); I find `DefaultOperatorNewCheck` to be insufficiently explanative. Maybe `DefaultOperatorNewAl

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri marked an inline comment as done. lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().g

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + balazske

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNe

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + aaron.bal

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + aaron.ballman w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong wrote:

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + aaron.ballman w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + lebedev.r

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong wrot

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong wrote:

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

martong added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:38 +return; + + ASTContext &Context = D->getASTContext(); Would it make sense to early return here if the language dialect is >= C++17 ? =

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske updated this revision to Diff 220628. balazske added a comment. Fixed the test, fixed problems in list.rst. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67545/new/ https://reviews.llvm.org/D67545 Files: clang-tools-extra/clang-tidy/cer

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske updated this revision to Diff 220625. balazske added a comment. Rebase and update according to comments. C++17 related changes not implemented yet (possible check for the called allocation function). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:53 +CheckFactories.registerCheck( +"cert-default-operator-new"); CheckFactories.registerCheck( balazske wrote: > aaron.ballman wrote: > > balazs

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:53 +CheckFactories.registerCheck( +"cert-default-operator-new"); CheckFactories.registerCheck( aaron.ballm

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added a comment. Thank you for working on this check! In D67545#1672106 , @balazske wrote: > C++17 makes things more difficult because the align is probably handled by > `operator new`, probably not, depending on the defined allocation func

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:49 + if (HasDefaultOperatorNew && OverAligned) +diag(NewExpr->getBeginLoc(), "using default 'operator new' with over-aligned

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:53 +CheckFactories.registerCheck( +"cert-default-operator-new"); CheckFactories.registerCheck( The checker

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske marked 3 inline comments as done. balazske added a comment. C++17 makes things more difficult because the align is probably handled by `operator new`, probably not, depending on the defined allocation functions. This can be observed only with a non clang-tidy checker (we could compute t

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

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

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

lebedev.ri added a comment. Some thoughts: 1. Docs missing 2. How does this play with C++17 aligned new? Assuming compiler/library support for that isn't broken ('bye, OSX!', ?), i'm not sure why it would be UB for C++17, see https://godbolt.org/z/kwxRbu vs https://godbolt.org/z/om-bR2 3. I'm n

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

balazske created this revision. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dexonsmith, steven_wu, dkrupp, xazax.hun, inglorion, mgorny, mehdi_amini. Herald added a project: clang. Added new checker 'cert-default-operator-new' that checks for CERT rule MEM57-CPP. Simple version.