Maybe just move StdCLibraryFunctionsChecker to core? (.apiModeling?) We officially don't support disabling core, so i guess it kinda solves the issue. Also all of our languages are C-based, these functions are present on all platforms (if any of those aren't, we could split them out and keep in unix).

Yay!

I suspect that other checkers may suffer from the same problem.

Maybe use text as well? It's hard to decipher color coding if you're not the one who introduced it.

Mm okay right. I wonder what were the plans for NSErrorChecker that required this.

You can retrieve any node from the ASTMatcher by .bind()ing sub-matchers to string keys and later retrieving them from the MatchResult dictionary by those keys. It's like a regex capturing groups (and, well, ASTMatchers also support backreferences to such groups, i.e. equalsBoundNode()).

Yup. Ugh.

Add a defensive check that prevents more crashes if the source type is not a class at all.

Hmm, how did i miss it?

Yeah, these globals were weird.

Add a bit more discussion into the comments.

Mm, i mean, that reason as good as well.

In D52133#1237312, @george.karpenkov wrote:

@NoQ Actually I agree with @baloghadamsoftware that it makes sense to have a separate test, as this functionality should be tested regardless of svalbuilder-rearrange-comparisons existence.

Should not it we have its own test in expr-inspection.c?

Address comments. Add more sanity checks and test them.

So trimmed graphs are in a different allocator?

We're getting pretty good at it ^^

Yay!

I hope we'll be able to come up with a reasonable default regex.

Or, wait, hmm, no, we can still de-duplicate the reports with a global set stored in the checker object. Because we would probably like to de-duplicate across different paths as well.

Hmm, i guess it's necessary to de-duplicate across multiple reports as well.

Hmm, so we're reporting the same region twice in the same bug report through different field chains, right? Why is a state trait necessary here? Maybe just de-duplicate them locally before throwing the report?

Looks good i guess. I'm glad this is sorted out^^

Yeah, i guess that's a good thing to know.

Looks as expected. I hope that assertion does actually hold. Should AST folks have a look as well?

Yay.

Looks great to me and pretty trivial, should somebody who's more familiar with this code take a look?

This would have helped me immensely with debugging temporary materialization support. Assuming that we dump these identifiers in symbols/regions as well.

Prevents bad centering.

Fair enough :)

What do you think about the following approach:

1. Define a lit substitution %diff_plist that resolves to diff -u -w -I "<string>/" -I "<string>.:" -I "version" -.
2. Update these flags in your downstream fork.

Comments always welcome!

I'm generally fine with landing the patch as long as the overall direction is chosen (and be moved towards) for how to safely identify the non-deterministic iterators (my previous inline question). This has to be addressed before we move out of alpha, but i believe we should pick at least some direction now, because i've very little idea about how to actually do that safely, and i'm worried that we'll get stuck in a dead end here.

Dunno, i guess you can just commit it.

Yay.

Test? (:

Yeah, something like that :)

Yup, either that or at least a smart pointer by value.

The interesting part here is that you immediately see in which order nodes were created, which is great for debugging exploration order.

I'm happy with the code.

I very very much appreciate this.

I actually thought it's just a different name for graphviz :/

Very easy:

Yay.

In D51300#1220537, @Szelethus wrote:

Would you be comfortable me commiting this without that assert

Ok, i'll get to the rest of the stuff a bit later (unless you pick it up).

In D51393#1218544, @george.karpenkov wrote:

In fact, generating a single "long" seems even better.

Yeah, i guess i should've split those up.

The code looks great.

In D51393#1217644, @aprantl wrote:

Just for consideration: The raw pointers in dumps are sometimes useful while in a debugger session, because you can cast a pointer and dump the object in the debugger.

Let's commit then?

Eg., let's test something like this, in both C and C++:

struct Point {
  int x, y;
};

I guess it's better than nothing.

Should be great as long as various analyzer UIs don't try to enforce this flag manually. Eg., scan-build seems to trust the driver here and doesn't expose it as an option.

The contract of this method is pretty unclear. I guess we may eventually have to bring the State back at some point, but i wouldn't mind having it shorter now.

Yup, looks correct to me!

Let's see what it finds, i guess.

Address comments.

Yay.

Why is this even a class?

Tried to address the regression with double inheritance by introducing a CXXDerivedObjectRegion, which is the opposite of CXXBaseObjectRegion, to represent such casts. Such region is a bit weird because it is by design bigger than its super-region. But it's not harmful when it is put on top of a SymbolicRegion that has unknown extent anyway.