[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

This revision was automatically updated to reflect the committed changes. Closed by commit rG59878ec8092b: [analyzer] Add path notes to FuchsiaHandleCheck. (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D70725?vs=231996=234952#toc Repository: rG LLVM Github

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. Thanks for the reviews! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great, thanks!~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 ___ cfe-commits mailing list

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:307 + std::vector> notes; SymbolRef ResultSymbol = nullptr; I will capitalize this name. CHANGES SINCE

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun updated this revision to Diff 231996. xazax.hun added a comment. - Only add note tag if we have actual notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. In D70725#1767942 , @NoQ wrote: > Mmm, right, i guess you should also skip adding the tag if the notes array is > empty. > > There might be more complicated use-cases (especially in `ExprEngine` itself) > where you may end up

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

NoQ added a comment. Mmm, right, i guess you should also skip adding the tag if the notes array is empty. There might be more complicated use-cases (especially in `ExprEngine` itself) where you may end up adding a tag even if the state doesn't change. For instance, when changing an

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. In D70725#1767673 , @NoQ wrote: > In D70725#1767643 , @Charusso wrote: > > > In D70725#1767579 , @xazax.hun > > wrote: > > > > > Just a small

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun updated this revision to Diff 231989. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 Files:

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:363-366 +if (() != && +() != && +() != ) + return ""; NoQ wrote: > Ugh, it sucks that we have to do this by hand. > > Why would

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

NoQ added a comment. Yay, it actually works! I only have minor comments and ignorable hand-waving now. In D70725#1767643 , @Charusso wrote: > In D70725#1767579 , @xazax.hun wrote: > > > Just a small side note. If

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

Charusso added a comment. In D70725#1767579 , @xazax.hun wrote: > Just a small side note. If the state was same as the previous we do not end > up creating a new ExplodedNode right? Is this also the case when we add a > NoteTag? It only happens for

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. You were right, the new API looks cleaner even in this case :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:362 } - C.addTransition(State); + const NoteTag *T = C.getNoteTag([this, notes](BugReport ) -> std::string { +if (() !=

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. Just a small side note. If the state was same as the previous we do not end up creating a new ExplodedNode right? Is this also the case when we add a NoteTag? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun updated this revision to Diff 231955. xazax.hun added a comment. - Use the new notes API for path notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

NoQ added a comment. > `note()` Of course i mean something like this: std::string text = note(); if (!text.empty()) return text; (and return something like `""` on the other return path in the inner lambdas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

NoQ added a comment. In D70725#1765981 , @xazax.hun wrote: > the same call might both acquire and release handles (there are such calls in > Fuchsia), so we might end up adding more than one note for a call for which > we would need to add more than one

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun marked 3 inline comments as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:261 + + // TODO: do we want to emit notes for escapes? + // do we want to emit notes for for error checks? I.e. on which

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. Sorry, my previous comment might not be clear. The point is, the same call might both acquire and release handles (there are such calls in Fuchsia), so we might end up adding more than one note for a call for which we would need to add more than one transitions. If

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun added a comment. I was thinking about note tags, but since we iterate on the argument/parameter pairs and might change the state in each iteration we would need to add a new transition after each change. I was wondering if using note tags would worth the cost (both in performance due

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

NoQ added a comment. So... note tags ? Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:261 + + // TODO: do we want to emit notes for escapes? + // do we want to emit notes for for error checks? I.e. on which

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

xazax.hun created this revision. xazax.hun added reviewers: NoQ, haowei. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. xazax.hun added a reviewer: Szelethus.