I read https://en.cppreference.com/w/cpp/language/structured_binding carefully, and there are a number of interesting rules that might deserve their own test case, even if this isn't the patch where you solve that issue, or believe that the solution handles it without the need for special case handling.

No need for post commit fixes, just general observations since I noticed them.

I tried poking this from a few directions, like nasty GNU extension types, ObjCObjectPointerType, but those seem orthogonal to this patch. Looks great! I'd wait for someone else's approval as well, as I try my best to pick up the thread.

LGTM

Fixes according to reviewer comments.

Very well :) Let's abandon this in its current state, I share this sentiment:

LGTM! You did check whether a missing doc field will actually trigger this error, right?

LGTM on my end, this is awesome!

Seems like all new files are missing the header blurb about the licence.

Nice!

In D120992#3364804, @NoQ wrote:

This check checks must-properties/all-paths properties. This has to be a data flow / CFG-based warning. I don't think there's a way around.

Remove a newline.

Can we reopen this if the code is not upstream at this time?

Sorry for the slack, I assumed this was accepted already. Thanks!

Fixes according to reviewer comments.

• Rename from .*Imprecise to .*AsWritten.
• Copy comments to relevant functions.

Cheers!

LGTM! Unrelated to this review, I don't think the term 'sink' is good in a warning message, are users expected to know what that is?

LGTM!

Sounds about right! Just a nit, otherwise LGTM.

Now that I remember, the ever so slightly different overloads of ProgramState::getSVal is a prime example I think. I always percieved that I have the means to invoke several of them at any point, but I never really knew which one. Though, to be fair, they were not documented particularly well (at least as I remember it).

In D119004#3297025, @steakhal wrote:

I strongly belive that this should be an overload to the existing 'matches' API. Maybe add a comment that prefer the other overload if can. But having an overload for that alread implies this anyway.

Move CallDescription specific changes to D119004.

Ping ^-^

Fix tests, mention that this is purely a heuristic.

First off, your patch is great, and I'm pretty sure we want it!

Add a default text, if another, unhandled unnamed identifier pops up.

LGTM!

I recognize the problem, but don't agree with the solution: this is a CallDescriptionSet at this point, not a CallDescriptionMap. Also, I just don't think this is the place to use template specialization, nor do I think we should keep the function name lookup, if we only care about whether the call is in the set or not.

I guess this constructor prevents us from to just put this into ArrayRef:

I'll intend to land this by friday unless there are objections!

Maybe we should change the name of this checker to NullConstructedString or something, instead of piling even more different kinds of warnings into it in the future. Or maybe not. In any case, the patch LGTM!

Add context.

Fix incorrect sectioning.

Clarify the summary.
Delete unnecessary includes.
More fitting iterator names in the test files.

Big yes to the idea, the config of choice as the first to document seems fine, but I'm not sure we should lecture the user right in the very first paragraph. How about a short description of what a FAM is in the first place, than examples of the 3 types of it, and why we assume that only the first and the second to be such by default. If you insist, a word of caution would be okay in the last paragraph.

Commited in rGee6913cc8317c08b603daed64b07a17a95ec926a to release/13.x.

rGfb4d590a622f4031900516360c07ee6ace01c5e6 should sort this out!

I'll attend to this ASAP. Thanks for the heads up!

Gonna land this in a day or two, regardless of whether its accepted! Please take a look if you have anything to object to!

I like everything I see here so far! As soon as those debug functions are in, the patch should land!

In D108753#2988632, @NoQ wrote:

This looks good!

I guess one way to make this even more conservative would be to match the variable inside the delete-expression to the one we expect to get deallocated.

In D108912#2982169, @RedDocMD wrote:

I suppose that the SmartPtrModelling patches from GSoC this year shouldn't be added since they only involve an alpha checker.

In D108912#2982169, @RedDocMD wrote:

I suppose that the SmartPtrModelling patches from GSoC this year shouldn't be added since they only involve an alpha checker.

In D108912#2972167, @steakhal wrote:

Maybe a couple other noteworthy commits:
efa7df1682c2859dabe3646ee7dc01e68629417f: better R-value tracking.
aa454dda2eed4e71081bc57b1f32dfce2486b177: fixing the modeling of std::bit_cast<>.

Fixes according to reviewer comments, cheers!

indent->intent

Lets go! Mind that you wrote "eazy" instead of "easy" in the revision name. But, if you are just being that "kewl", I don't mind!