[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2023-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision now requires review to proceed. Herald added subscribers: carlosgalvezp, StephenFan. Herald added a project: All. This review seems to be stuck/dead, consider abandoning if no longer relevant. Repository: rG LLVM

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D76229#1929453 , @martong wrote: > Please note about the alignment requirements that they are not checked in the > static analyzer for a reason. To track the alignment data across the data > flow, probably we should modify the ana

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-18 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Please note about the alignment requirements that they are not checked in the static analyzer for a reason. To track the alignment data across the data flow, probably we should modify the analyzer's core structure (e.g. `MemRegion`) and some modeling checkers like the `

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-18 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment. In D76229#1926818 , @Charusso wrote: > In D76229#1925363 , @f00kat wrote: > > > In D76229#1925268 , @aaron.ballman > > wrote: > > > > > Have you cons

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D76229#1925363 , @f00kat wrote: > In D76229#1925268 , @aaron.ballman > wrote: > > > Have you considered making this a static analyzer check as opposed to a > > clang-tidy check? I thin

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Sure np, was just curious^^ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76229/new/ https://reviews.llvm.org/D76229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-17 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment. NoQ, I don`t want to say that existing static alalyzer is bad written or something. I just noticed that it does not yet support arrays and alignment. Anyway thank you for your detailed feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-17 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a reviewer: martong. NoQ added a comment. Herald added a subscriber: rnkovacs. First of all, the static analyzer doesn't warn on everything at once; it stops the analysis after a fatal error (such as UB) is found, so it doesn't explore the execution path further. This is the reason why

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-17 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment. In D76229#1925948 , @lebedev.ri wrote: > In D76229#1925371 , @f00kat wrote: > > > In D76229#1925360 , @lebedev.ri > > wrote: > > > > > This seems to

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a reviewer: NoQ. lebedev.ri added a comment. Herald added a subscriber: Charusso. In D76229#1925371 , @f00kat wrote: > In D76229#1925360 , @lebedev.ri > wrote: > > > This seems to be already handle

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment. In D76229#1925360 , @lebedev.ri wrote: > This seems to be already handled by clang static analyzer? > (`clang-analyzer-cplusplus.PlacementNew`) > > :19:5: warning: Storage provided to placement new is only 2 bytes, > whereas the

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment. In D76229#1925360 , @lebedev.ri wrote: > This seems to be already handled by clang static analyzer? > (`clang-analyzer-cplusplus.PlacementNew`) > > :19:5: warning: Storage provided to placement new is only 2 bytes, > whereas the

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat added a comment. In D76229#1925268 , @aaron.ballman wrote: > Have you considered making this a static analyzer check as opposed to a > clang-tidy check? I think using dataflow analysis will really reduce the > false positives because it will be a

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. This seems to be already handled by clang static analyzer? (`clang-analyzer-cplusplus.PlacementNew`) :19:5: warning: Storage provided to placement new is only 2 bytes, whe

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Have you considered making this a static analyzer check as opposed to a clang-tidy check? I think using dataflow analysis will really reduce the false positives because it will be able to track the allocation and alignment data across control flow. Repository:

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Also fix the test case, premerge found a failure Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:23 + + for (const Stmt *Child : TheStmt->children()) { +if (const auto *TheDeclRefExpr = dyn_cast(Child))

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:43 +} + + Unnecessary empty line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76229/new/ https://r

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat updated this revision to Diff 250556. f00kat added a comment. Fix 'const auto*' in function getDescendantValueDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76229/new/ https://reviews.llvm.org/D76229 Files: clang-tools-extra/clang-tid

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat updated this revision to Diff 250555. f00kat added a comment. 1. Static functions instead of anonymous namespaces. 2. const auto*. 3. Added double back-ticks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76229/new/ https://reviews.llvm.org/

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:19 + +namespace { +const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) { Please use static instead of anonymous namespaces for functions. See

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat created this revision. f00kat added reviewers: aaron.ballman, alexfh. f00kat added projects: clang, clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Added new checker 'cert-mem54-cpp' that checks for CERT rule MEM54-CPP. I have troubles writing English descripti