Fix unused include.

Ouch, forgot to accept it. Here you go!

On second thought, maybe let's restrict the scope to just bool conversions (don't model .get() and .release() yet) but enable the check by default immediately?

Add space before \.

Separate the canonicalization change that unbreaks body farms into a separate patch.

Fall back to the previous std::vector<std::unique_ptr<NoteTag>> approach.

In D58367#1462026, @Szelethus wrote:

Hmmm, interesting. Could there be an issue with NoteTag not being trivially destructible?

(also fix typo in the function name)

Don't canonicalize the decl in the body farm. The decl supplied by the AnalysisDeclContext is already the correct one (and not necessarily the canonical one).

In D60808#1469734, @NoQ wrote:

Hmm, i think i'd love to know why doesn't the uninitialized variable checker fire on the if-statement as farmed by the body farm:

Passing arguments to this whole body farm thing doesn't work. It builds the body for the declaration on line 4 but then calls the declaration on line 5, and parameter variables in the synthesized body don't match parameter variables of the call, so it cannot read argument values :/

Yup, i confirm that this improves discoverability of this feature :) Maybe it deserves its own .rst doc, like FileCheck, but for now doxygen seems to be the best source of truth on how to use -verify and i consult it regularly.

The actual "Behold!".

Fxd.

Hmm, i think i'd love to know why doesn't the uninitialized variable checker fire on the if-statement as farmed by the body farm:

592   // Signature:
593   // _Bool OSAtomicCompareAndSwapPtr(void *__oldValue,
594   //                                 void *__newValue,
595   //                                 void * volatile *__theValue)
596   // Generate body:
597   //   if (oldValue == *theValue) {
598   //    *theValue = newValue;
599   //    return YES;
600   //   }
601   //   else return NO;

(closing brace accidentally omitted in the original comment as well)

Fxd constructors and destructors.

In D60732#1469026, @jdenny wrote:

The original patch added documentation to -cc1 -help. Doxygen for VerifyDiagnosticConsumer is another possibility except it currently doesn't mention command-line usage at all. Maybe it should?

Fix comments.

Yay!! I'm happy with this change and redirected all my wrath on its dependencies.

Let's decide whether we want to display the new notes in the text output. @alexfh: Given that clang-tidy is the primary consumer of the analyzer's text output mode, would you rather have or rather not have these new note:s in the text output? (scroll to the bottom of D53076 to see how they look). In my opinion, i'd rather have them in text output (because text output is hard to navigate, and having the necessary information exactly where you need it would probably be helpful), but rather not have them in tools that parse clang-tidy notes and stuff them into a source code view in some GUI (such tools do exist, right?), so im confused. I'd also prefer to have them be off in text output in tests, because such tests usually test checker-specific notes and having more checker-inspecific notes in the tests is more annoying than useful. But that's pretty minor; i'll be happy to write them down every time i write a test if they are believed to be a good thing in text output in general.

In HTML reports background-color: LemonChiffon was too light, changed to PaleGoldenRod.

Why alpha? Because it needs a bug visitor to explain what's going on when it suddenly returns a note. Because it's going to be a bugtype-inspecific note that is emitted by a checker, i want to implement it via note tags (D58367), otherwise it's quite a layering nightmare to set up communication between core and checker visitors (especially given that we'd want to attach a new move visitor immediately after emitting the note).

Whoops, copy-paste error.

Fix test file name in the top comment.

This new approach is clearly useful to other checkers as well, not only the Taint checker. I believe we should strongly consider generalizing it somehow, it's just too awesome to restrict to a single checker.

Reverted in rC357827 because clang-tidy was using this function in some of its checks. I'll have a look at if it is also affected by the bug or was already using a workaround.

Address comments. Thanks!

I guess i'll commit, given that i don't know anybody else who uses these functions anyway.

Reverted D57230 in rC357620.

Mmm. I'm also pretty pinned down (it's seasonal), so i'm thinking of temporarily reverting :( until one of us gets to fixing the accidental effect on escaping, 'cause it just keeps biting us. Like, i wholeheartedly appreciate the work and i have already noticed a few times how it makes things better and loved it, it just seems to accidentally have something missing that nobody could predict, and i'll be super eager to get it back in.

In D60112#1451198, @Szelethus wrote:

Woah, the code looks amazing, cheers on the refactoring! I'll be honest, I'm struggling a bit with the sentence "we're now in the top frame". In order, I don't understand what does

• we
• now
• in the top frame mean. "Top-level argument" is another one -- Do we have precise definitions for there terms?

(whoops, forgot Alexey)

Looks great, thanks!

Re-reopen after the Diffusion auto-close.

Reverted in rC357332!

This other example seems to work and looks relatively similar, just with a different checker causing the visitor to be attached.

Cleaned up formatting a bit.

Also, this is kinda weird. According to my logic, we should have written Assuming 'i' is equal to 4294967295 because that's what the user will see in the macro popup. However, that's incorrect for the same reason: i is an int, while 4294967295 doesn't fit into an int, so they can never be equal. Writing Assuming 'i' is equal to UINT32_MAX is incorrect for the same reason, and the current message is the only one that's technically the truth.

Mmm, that *is* an Assuming... piece, i.e., this is the same code, just the structure of macros is more complicated than usual.

In D58573#1437749, @Szelethus wrote:

Ping, @NoQ, if we settled on optin.cplusplus, would you be fine with this patch?

On second thought, dunno. In the scan-build macro preview it wouldn't show you UINT32_MAX anyway. Maybe let's keep this behavior.

I think the right thing to do here is "look at the immediate macro; while it expands exactly to our original expression, look at what it is an expansion of; write down the last macro we've reached". My code now gives up whenever we stop expanding to the original expression, but it should write down the highest macro it has reached instead.

An elegant solution for a more civilized age. Unfortunately, doesn't preserve the UINT32_MAX macro in the newly added test - i'll try a bit harder to preserve it. Relies on D59977 to work.

Instead of having those as events similar to "Assuming", we could turn the new "Knowing" pieces into floating pop-ups - imagine you hover the mouse over a condition foo() and it says "foo() is false". That is, instead of PathDiagnosticsEventPiece, a new kind of piece can be introduced that is shown in scan-build as a pop-up similar to macro pop-ups. Like this:

What i was trying to say with my last comment is that i guess i'd rather go for option (1) because with that getRange() remains the single source of truth, which is comfy.

Add a test for taint dumps.

*un-forgets to actually post comments*

I agree that we shouldn't try real hard to fix our code until we have a clear "party line" on how exactly do we name our variables (and ideally also how do we invalidate our caches), but having variables and fields consistent with each other makes it look much better imo, so why not.

Address comments. Fix printing taint in state dumps.

Hi, i wanted to squeeze in D59861 somewhere in the middle of your work, would you mind?
I'll definitely have a look at your patches soon :)

(whoops sry nvm typo in reviewer list)

Remove memoization for now. Address a few inline comments. I'm mostly done with this first patch, did i accidentally miss anything?

(the commit message shouldn't have said "CFG", sry)