Friendly ping @gkistanova. What do you think about this modification? Also, should we add our buildslave to the silent (if I remember correctly it is called staging), or the reporting buildmaster? Thanks!

Revert unnecessary clang-formating of AnalysisConsumer.cpp

I could greatly simplify the API of getCrossTUDefinition by injecting the threshold value in the constructor. A minor drawback with this approach is that testing code is a bit more complicated, as I had to patch the CompilerInstance to override the AnalyzerOption value for CTUImportThreshold. Thanks for the suggestion!

Apply review suggestions by Xazax

In D59934#1449221, @Szelethus wrote:

Hmm. Is your clang recent enough to contain @bruntib's patch D57892? Is it possible that this patch solves the same issue? With this patch applied, are you able to get a macro expansions from a different TU in the plist output, or does this patch only resolve the regression?

In D59934#1449109, @Szelethus wrote:

I would still like to learn more about this issue. I am somewhat afraid that the our macro expansion is faulty. Can you provide a stacktrace maybe?

In D59934#1446059, @lebedev.ri wrote:

Test?

Just have a bit more context, I have the following information from a debug session at the execution point of the unreachable:

In D59934#1446736, @Szelethus wrote:

In D59934#1446051, @gamesh411 wrote:

Hi!

This issue came up during the generation BugReports of BugPaths containing macro-expansions, where the spelling location and expansion locations were in different files.
With this change, we make such SourceLocations comparable by their FileIDs.

Do you mean regular macro expansions or the one we're generating in the plist output via -analyzer-config expand-macros=true?

This issue came up during the generation BugReports of BugPaths containing macro-expansions, where the spelling location and expansion locations were in different files.
With this change, we make such SourceLocations comparable by their FileIDs.

ping @a_sidorin @shafik

ping @a_sidorin

Updated option handling location to use AnalyzerOptions instead of CC1

During CTU analysis, not only StructuralEquivalence but the main implementation of ASTImporter can also emit ODR-related diagnostics. After some consideration, I have chosen not to make these dependent on a switch, as ASTImporter is not nearly as widely used as StructuralEquivalence (thru Sema), and it would also complicate the code unnecessarily.
What do you think?

• Revert member order to original

This revision is the alternative of the abandoned D55646.
Now Sema uses the old behavior of emitting ODR errors while merging and importing is done in a more lenient way.

I am creating a new revision that keeps the old handling of ODR violations the same when used by parts of the Sema, but emit warnings when used by the ASTImporter.

I am going to abandon this modification, as setting ODR violations as warnings, seems like a change that could have unforeseen consequences.

Remove unnecessary ToTU variable from test case.

Ping.

Hey Aleksei!
Thank you for the input! ODR violations being warnings would be beneficial from the code maintenance point of view, as we would not have to resort to duplicate some (if not most) of them as errors. There is also a flexibility advantage in the diagnostics, as warnings can be propagated to error level or suppressed, whereas errors cannot be "demoted" (AFAIK). So while I am not opposed to providing an error AND a warning for these kinds of violations, if the SEMA or other modules do not need them to be explicitly defined as errors (for I don't know... tooling support or other reasons), then it would be cleaner to only have warnings for these.

In order to be able to handle ODR-related diagnostics with command line options, these diagnostics were moved from Error category to Warning. What are your thoughts?

In my opinion, after migrating relevant test cases from D33826, this is ready.

Hi!
Thanks for your reviews, although I haven't been active for some time now.
I personally do not have commit rights, so could someone else take care of it?

I have implemented the std::transform. The previous version used std::for_each because the iterator for enum declarations was not a random access iterator, but it turned out that I can solve this problem via std::distance. Thanks for sticking to your opinion on this one, because of it I could learn something new.

Ping. @NoQ would you please have a look? Thanks!

As for the the loss of precision problem, in the special case of char the size of char is known. However does the analysis have the necessary information in this stage to know the size of an int for example? I found bit-width specifying information in the llvm::Type class which is used in the code generation phase. It could be done by checking on a per type basis, but then again, it could possibly lead to false positives. Correct me if I am wrong.

Applied most of the suggested changes, thanks for all the insights!

After experimentation the following AST difference between the mock and the standard library implementation still stands (which necessitates the special handling of the complex manipulators). Example:

Fixed the naming convention issues. Also applied the suggested modifications inside the overridden checker method.

Update diff.

Update diff.

Hello,
This checker was developed indeed with internal usage in mind. It should not necessary be added as a default checker. However I have run it on the boost-1.63.0 codebase, and there some some mildly interesting findings in examples and tests. There is also a true positive result in the core codebase.