Reverted in rC349233 - fails on a Windows buildbot (http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/2642/steps/annotate/logs/stdio).

These are boring, i guess i just commit.

Rebase!

In D55697#1331001, @Szelethus wrote:

We could also add an assert to getOwningEngine I guess.

Yup, squash these! Even if there was a plan to use ProgramState separately for some other sort of analysis that doesn't involve a sub-engine, it is currently long forgotten.

Thanks! Neat test.

Rebase.

Fxd.

I approve this for Static Analyzer. As long as this is the correct thing to do in the compiler, Static Analyzer should ideally behave similarly to the compiler. Exposing portability bugs that would show up on other compilers is generally less important than behaving similarly to Clang itself.

Sure, np.

Yup, looks good, and i keep passively cheering for this project to be successful.

In D46421#1121080, @r.stahl wrote:

It seems like a good idea to not do that, since non-const values are not used. It might become useful if we ever do some kind of straight line execution from static initialization to main.
However for structs it is enough if one of their fields is declared const.

In D55388#1322601, @xazax.hun wrote:

Hm. I wonder if it would also make sense to model e.g. the get method to return nullptr for moved from smart ptrs. This could help null dereference checker and also aid false path prunning.

Thanks, looks perfectly sane to me!

I poked Devin offline. It kinda seems to both of us that Sphinx is great for documentation that is updated regularly, but our fancy front page should probably stay. Similarly to how we still have the llvm.org front page, despite all the documentation. It's kinda atmospheric and friendly and people like it and/or are used to it.

Added a test in rC349000.

In D53280#1325375, @Szelethus wrote:

I guess the solution would be to check whether there are any user supplied flags with "analyze" substring, and add the compatibility flag then. It is possible if not probable that a non-static-analyzer related flag with a name like that will eventually be added, but I guess we can live with it.

That is, it is modified within the recursive call.

I think that it is modified, at least, when Memoization[C] is evaluated. The object needs to be created and put into the map in order to obtain a reference to it.

Hmm, i think i get it. Cached is a reference. Changing Memoization will invalidate references because DenseMap doesn't provide the respective guarantee.

Add documentation, fix checker help message.

Let's commit without a test because of the urgency, and add the test soon.

Should we also pass CheckerManager into shouldRegister...? Or is it entirely useless?

Thx!

Looks great!

Sounds reasonable.

Also you might need to rebase on top of D55400.

Fix typo.

Generally, because the primary user of checker names is clang-tidy, the correct name to display is the name of the specific checker that the user needs to disable in order to get rid of the warning.

I'm seeing no problems with this patch and i'm happy that we're replacing hacks with well-defined patterns :)

Aha, ok, so what's the final procedure now to register a checker that's artificially split in two? Something like this, right?

def CommonModel : Checker<"Common">,
  HelpText<"Doesn't emit warnings but models common stuff.">;

Yeah, this part looks easy :)

Nice!

Fxd!

Fxd.

I just generally wish for a function that would return the most specific known range for the given arbitrary symbol. This is something we can put, for example, into visitor path notes.

In D54438#1315953, @Szelethus wrote:

• ✔️ There are in fact a variety of checkers that contain subcheckers like MallocChecker, which I think is all good, but the concept that if a subchecker is enabled it only sort of registeres the main checker (in MallocChecker's case it enables modeling, but not reporting) is very hard to follow. I propose that all such checkers should clearly have a base, called something DynamicMemoryModeling, or IteratorModeling etc.

In D53701#1318566, @baloghadamsoftware wrote:

In D53701#1315242, @NoQ wrote:

When using iterator adapters of the STL the bugs may also be filtered out by the analyzer if this option is enabled.

I think the idea is great, thanks!

Burn it.

Add one more important case: std::optional. When moved, it moves the object within it into the new optional, if any. So the object is left in unspecified state, but the state of the optional itself is well-specified.

Fxd :)

^^

In D55150#1321793, @jyknight wrote:

In D55150#1321759, @george.karpenkov wrote:

Using -Xclang is the only way to pass options to the static analyzer, I don't think we should warn on it.

Well,, that seems unfortunate if we have the only supported interface for the static analyzer be an internal interface. Perhaps it can be given a different option? Even discounting this change, I that seems like it would be appropriate.

Address comments :)

Convert the pointer to a reference.

It's a flag that was removed but old soft doesn't know that it was removed, and it won't be fixed because it's old. I guess a typo would have caused the same problem.

Ugh. We broke backwards compatibility when an invalid -analyzer-config is specified but -analyze isn't specified. Yes, people are accidentally relying on that :/ :/ :/

Hmm, i shouldn't have included shrink_to_fit. It definitely keeps the object in unspecified state if it is already in unspecified state.

Looks great, thanks!

In D54560#1302051, @MTC wrote:

The "moved-from" terminology we adopt here still feels a bit weird to me, but i don't have a better suggestion, so i just removed the single-quotes so that to at least feel proud about what we have.

I am personally fine with this terminology, this checker corresponds to the cert rule EXP63-CPP. Do not rely on the value of a moved-from object, and moved from is also used in many places in CppCoreGuidelines.

Double-checked - this terminology does indeed come from the Standard:

I'm glad you brought this stuff up, found two more bugs to fix.

Add a TODO.

In D54563#1306358, @Szelethus wrote:

This suggests to me that list1.splice(list1.begin(), std::move(list2)) leaves list2 in a valid, well-defined, but empty state.

Address comments!