aaron.ballman added a comment. In D92039#2462995 <https://reviews.llvm.org/D92039#2462995>, @vsavchenko wrote:
> In D92039#2462889 <https://reviews.llvm.org/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 check could determine on its own. > > I mean, of course, it doesn't have real inter-procedural analysis. But if > everything is annotated or conventional - it is identical to inter-procedural > case (like in tests with `indirect` in their names). Okay, I'm glad to know I wasn't misunderstanding something there! >> I'm not worried about the leniency and I'm sorry if it sounds like I'm >> implying that it has to be inter-procedural to land -- that's not the case. >> What I am worried about is that the way the user runs the frontend is very >> different than the way the user runs the static analyzer, and migrating the >> warning from the FE to the static analyzer could leave us with problems. If >> the intention is that this check will live in the FE as-is (not getting >> inter-procedural support), then that's fine. But if the intention is to >> extend the check in the future in a way that might need it to move out of >> the FE and into the CSA, then I think the check should live in the CSA from >> the start (even if it doesn't do inter-procedural analysis). It's not clear >> to me whether "in the first version" implies that you expect a subsequent >> version to live in the CSA or not. > > No, we don't plan on moving it to the CSA. As I mentioned above, for > hardening we can simply warn if `called_once` parameters leak into functions > we don't know about (not annotated or not following conventions). Okay, thank you for verifying. The attribute bits of the patch LG to me (aside from a minor testing request). Someone else should validate the actual analysis bits. ================ 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 ---------------- Can you also add tests that the attribute accepts no arguments and only appertains to parameters? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92039/new/ https://reviews.llvm.org/D92039 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
