- User Since
- Sep 3 2015, 9:16 AM (214 w, 4 d)
Mon, Oct 7
Wed, Oct 2
Yup, optional sounds perfect.
Tue, Oct 1
Mon, Sep 30
I actually like the idea, it makes it consistent with other maps. But you'll need to clean up memory management here. Given that you can't modify the state in getDynamicTypeInfo(), i guess you'll have to resort to smart pointers.
Sat, Sep 28
Fri, Sep 27
Thank you, fantastic finding!
+@Szelethus because i'm a bit out-of-the-loop on plugins: i very vaguely remember that we decided to put them into tests(?)
Mon, Sep 23
Yeah, i think we should avoid such peeking and instead try to do everything in one pass. I.e., if we need to peek at the node above us, just make a visitor that delays the decision until it has precisely the information it needs. I guess i'll be slooowly moving in this direction.
Thanks! I like how it makes the option transparent to fellow developers by having it show up in the -### run-line.
Accept as soon as Alex's comment is addressed somehow :)
Thu, Sep 19
Is it just me or phabricator somehow refuses to display the changes since the last diff here? That's probably the commit diff is involved somehow. So i'm confused what exactly has changed >.<
Wed, Sep 18
You can go one step further to have a CallDescriptionMap, like in D62557. This would replace the whole chain of ifs with a map lookup (which is currently still a chain of ifs under the hood, but a lot less code anyway and we'll probably optimize it under the hood later).
Btw, evalCall is not deprecated. In fact, there are no alternatives for it in many cases.
That's a nice twist, i like it!
Sep 13 2019
FTR, we already have a similar Static Analyzer check, eg.: https://github.com/llvm-mirror/clang/blob/release_80/test/Analysis/dispatch-once.m#L15
Sep 12 2019
Umm, this test fails for me locally because i see osx checkers displayed in the list. Which is weird because they're supposed to be enabled based on the target platform, not on the host platform.
SimpleStreamChecker is a historical tutorial example, i don't think we should be updating it other than for modernizing checker API use. It was supposed to handle a few simple examples correctly but it wasn't supposed to support all sorts of different APIs.
I'd like the test cases to actually demonstrate the correct use of the filters and the correct behavior of the Analyzer when the filters are annotated correctly, but it looks to me that they either demonstrate behavior when the annotation is not used correctly, or we disagree about how the taint should work in the first place. Testing the behavior when the annotation is misplaced is fine (with enough comments and probably FIXMEs where applicable), but "positive" tests are more valuable because they are the actual common cases (hopefully).
Sep 11 2019
Aha, nice, thank you!!
Unforget to actually move the consumer implementations to libAnalysis, not just their headers.
Clean up a tiny bit of dead code.
Rename BugType to WarningMessage, add comments.
Unforget to do the same for the text consumer. As a side effect, the factory function for the text consumer is no longer special, which will be less confusing when put in libAnalysis.
Hmm, does anybody want me to write an example tool that emits path diagnostics but doesn't link to libStaticAnalyzer*?
Sep 10 2019
Sep 9 2019
I have mixed feelings. Removing boilerplate is good, but the very fact that we're legalizing this pattern indicates that our checkers will keep bloating up, while i always wanted to actually split them instead (like, make sub-checkers into their own separate classes, possibly spread out into different files, kinda micro checkers as opposed to monolithic checkers (?)). But i guess it's about whoever gets things done first :)
Sep 6 2019
Yup, thanks, this is really nice!
Sep 5 2019
Fix most comments.
Sep 4 2019
You don't have commit access, right? Do you want me to commit this patch for you?
I wish i had time to fix D55683 so that i could have you write tests for it :)
Sep 3 2019
Make fix-it hints attachable to any PathDiagnosticPiece, rather than to PathDiagnostic as a whole. This basically allows attaching a fixit to any note in the report. For now the plist consumer only supports attaching fixits to the warning itself, to path event pieces, and to extra note pieces; it's completely unclear what's the benefit of attaching fixits to other kinds of pieces.