[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2021-01-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:822 + /// calling the parameter itself, but rather uses it as the argument. + template + void checkIndirectCall(const CallLikeExpr *CallOrMessage) { NoQ wrote: > vsavchenko w

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-24 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Amazing, thank you. I'm happy with the analysis and i have nothing more to say really :) Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:822 + /// calling the parameter itself

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-23 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:163 + } + static bool canBeCalled(Kind K) { +return (K & DefinitelyCalled) == DefinitelyCalled && K != Reported; NoQ wrote: > "Can be called" sounds like "Someone's allowed

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-23 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/include/clang/Analysis/Analyses/CalledOnceCheck.h:32 +/// \enum SwitchSkipped -- there is no call if none of the cases appies. +/// \enum LoopEntered -- no call when the loop is entered. +/// \enum LoopSkipped -- no call when th

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-22 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D92039#2463039 , @vsavchenko wrote: > Add one gigantic comment on status kinds and the reasons behind some of the > design choices This was freakin' awesome, thanks a lot. With all the background in place, the rest of the patch w

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-18 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments. Comment at: clang/test/SemaObjC/attr-called-once.m:7 +void test2(double x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}} + +void test3(void (*foo)() CALLED_ONCE); // no-error ---

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D92039#2462995 , @vsavchenko wrote: > In D92039#2462889 , @aaron.ballman > wrote: > >> Your test cases suggest that this is not inter-procedurally checked; it >> seems to require

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-18 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D92039#2462889 , @aaron.ballman wrote: > Your test cases suggest that this is not inter-procedurally checked; it seems > to require all of the APIs to be annotated in order to get the same effect > that an inter-procedural

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D92039#2462255 , @vsavchenko wrote: > In D92039#2458392 , @aaron.ballman > wrote: > >> > > > >> There have been efforts to add cross-TU support to the static analyzer, so >> I'm

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-18 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 312741. vsavchenko added a comment. Detect all conventions for captured parameters as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/cla

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-18 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D92039#2458392 , @aaron.ballman wrote: > > There have been efforts to add cross-TU support to the static analyzer, so > I'm less worried about cross-TU inter-procedural bugs over the long term as I > would expect that

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D92039#2458042 , @vsavchenko wrote: > That was a tough choice whether we should do it in the analyzer or not. > The analyzer check would've been easier in terms of existing infrastructure, > path sensitivity, and IPA. It

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-16 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D92039#2457867 , @aaron.ballman wrote: > Is the attribute considered to be a property of the parameter or a property > of the function the parameter is declared in? e.g., > > void someOtherFunc(void (^cb)(void)) { > i

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Is the attribute considered to be a property of the parameter or a property of the function the parameter is declared in? e.g., void someOtherFunc(void (^cb)(void)) { if (something()) cb(); } void barWithCallback(void (^callback)(void) __attribut

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-16 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 312155. vsavchenko added a comment. Add more info to the docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analyses/CalledOn

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-16 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 312149. vsavchenko added a comment. Turn off conventional check heuristic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analy

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5095 +Clang implements a check for ``called_once`` parameters, +``-Wcalled-once-parameter``. It is on by default and finds the following +violations: Comme

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 311930. vsavchenko added a comment. Introduce a suppression for double call warning by nullifying the parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 File

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 311903. vsavchenko added a comment. Fix minor things Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analyses/CalledOnceCheck.h

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-15 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 311898. vsavchenko added a comment. Refine conventions for completion handlers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-14 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 311565. vsavchenko marked 4 inline comments as done. vsavchenko added a comment. Add documentation for new attribute and fix review remarks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://re

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-11 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko marked an inline comment as done. vsavchenko added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1774 + let LangOpts = [ObjC]; + let Documentation = [Undocumented]; +} aaron.ballman wrote: > No new undocumented attributes, please. Su

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D92039#2441992 , @vsavchenko wrote: > @aaron.ballman Can you please take a look at the attribute side of this? Thank you for your patience, sorry it took me a while to get to this! Comment at: clang/in

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-12-09 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. @aaron.ballman Can you please take a look at the attribute side of this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 ___ cfe-commi

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-30 Thread Ravi via Phabricator via cfe-commits
ravikandhadai added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:889 + + /// Return true if th analyzed function is actually a default implementation + /// of the method that has to be overriden. typo: th -> the Repository: rG LLVM Gi

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 307594. vsavchenko marked an inline comment as done. vsavchenko added a comment. Add another comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/i

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko marked 2 inline comments as done. vsavchenko added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:76 + +// Defined elements should maintain the following properties: +// 1. for any Kind K: NoReturn | K == K NoQ wrote: > M

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-25 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 307569. vsavchenko added a comment. Herald added a subscriber: jdoerfert. Add more comments and fix another test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Fil

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. A few random comments here and there as i slowly wrap my head around the overall analysis algorithm. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:47 +using CFGSizedVector = llvm::SmallVector; +constexpr llvm::StringLiteral CONVENTIONAL_NAME = "comple

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko updated this revision to Diff 307408. vsavchenko added a comment. Fix tests and a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 Files: clang/include/clang/Analysis/Analyses/CalledOnceChe

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment. In D92039#2414314 , @Quuxplusone wrote: > Probably irrelevant comment from the C++ world: If I needed this concept in > C++, I'd probably piggyback on the existing semantic analysis of `std::move`: > I'd design a `class OnceCa

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:1223 +// We consider '(void)parameter' as a manual no-op escape. +// It should be used to explicitly tell the analysis that this paramater +// is intentionally not called on this pat

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. Probably irrelevant comment from the C++ world: If I needed this concept in C++, I'd probably piggyback on the existing semantic analysis of `std::move`: I'd design a `class OnceCallable` with an `operator() &&` that both called and nulled-out its object parameter,

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision. vsavchenko added reviewers: aaron.ballman, dexonsmith, rsmith, Bigcheese, rjmccall, NoQ, doug.gregor, ravikandhadai, dcoughlin. Herald added subscribers: cfe-commits, Charusso, mgorny. Herald added a project: clang. vsavchenko requested review of this revision. T