[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa97648b93846: [analyzer][StdLibraryFunctionsChecker] Add better diagnostics (authored by martong). Changed prior to commit: https://reviews.llvm.o

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. In D79431#2265155 , @Szelethus wrote: > In D79431#2263693 , @martong wrote: > >> In D79431#2263690 , @martong wrote: >> >>> What if we'd add a `toStr

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D79431#2263693 , @martong wrote: > In D79431#2263690 , @martong wrote: > >> What if we'd add a `toStri

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. In D79431#2263690 , @martong wrote: > In D79431#2215610 , @Szelethus wrote: > >> Ah, okay, silly me. Didn't catch that. Then the message is OK for now. >> >> edit: Describing //how// the vio

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. In D79431#2215610 , @Szelethus wrote: > Ah, okay, silly me. Didn't catch that. Then the message is OK for now. > > edit: Describing //how// the violation happened might be a valuable for > development purposes as well. What if we

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

Szelethus added a comment. Ah, okay, silly me. Didn't catch that. Then the message is OK for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 ___ cfe-commits

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

balazske added a comment. There is a TODO comment at the error message, it needs improvement. The current form is still something for developers, not for end users. For final version I would accept textual descriptions (as mentioned above), not names like "BufferSize" and words like "ArgN" insi

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Tests c: I'm still not a huge fan of the warning message. Now it describes //what kind// of constraint was violated, but not //how// (too large? too small?). Also, while I res

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong updated this revision to Diff 280376. martong marked 3 inline comments as done. martong added a comment. - Use Twine Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 Files: clang/lib/StaticAnalyzer

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323 +std::string("Function argument constraint is not satisfied, ") + +VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo()); if (!BT_Inva

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong marked 4 inline comments as done. martong added a comment. In D79431#2020951 , @Szelethus wrote: > Sure, this is an improvement because we display more information, but I'd > argue that it isn't really a more readable warning message :) How about

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong updated this revision to Diff 280368. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecke

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

steakhal added a comment. Could you add some tests demonstrating the refined diagnostics of the checker? It would help to validate the quality of the emitted diagnostics. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:88 /// obviously uint32_t s

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

balazske added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323 +std::string("Function argument constraint is not satisfied, ") + +VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo()); if (!BT_Inv

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

baloghadamsoftware added a comment. In D79431#2020951 , @Szelethus wrote: > Sure, this is an improvement because we display more information, but I'd > argue that it isn't really a more readable warning message :) How about > > th argument to the call to

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

baloghadamsoftware added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:323 +std::string("Function argument constraint is not satisfied, ") + +VC->getName().data() + ", ArgN: " + std::to_string(VC->getArgNo()); i

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

xazax.hun added a comment. Also, I see no tests :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

Szelethus added a comment. Sure, this is an improvement because we display more information, but I'd argue that it isn't really a more readable warning message :) How about th argument to the call to - cannot be represented with a character - is a null pointer - ... , which violates the funct

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

martong created this revision. martong added reviewers: Szelethus, NoQ, baloghadamsoftware, balazske, steakhal. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a projec