Rebase to master

• Add FIXME

Rebase to master

Ping

Ping

@shafik Thanks for the review!

Rebase to master(trunk)

Thanks for the review Alexei!

• Remove numbers when possible
• Use disabled tests instead of commented out tests

How is #if cplusplus >= 201103L qualitatively different from #ifndef NDEBUG or #if MYLIB_ABI_VERSION==2 or #if DATE__ == "2018-04-01"?

This project is about writing a fuzzer to proactively discover these

ASTImporter bugs and provide minimal reproducers which make understanding
and fixing the underlying bug easier.

Ping.
Please raise your objections if you have any until the 18th of February (that date I am going to commit if there are no objections). Also, please let me know if you find this deadline too strict.

Ping

• Revert comment change
• Add lang_dialect_mismatch and stats for that

• Add comment above ASSERT_NE

I think you should change back to getInit()

I am not entirely sure about this because the initalizer may not be attached to the canonical decl. getInit() gives the initializer of one given specific Decl, however, getAnyInitializer() searches through the whole redecl chain.

Previously this was not required since all VarDecls were canonical. Not sure if this change was intended. I did some digging, but am not familiar enough with the code base to figure out what changed. Does anyone have an idea about this?

Ping

• Move hunks into this patch from parent patch

Update: I just removed the unrelated changes.

• Remove unrelated hunks

@martong have you had a chance to look at this some more? We ran into another problem that this fixes partially.

I didn't have time to deal with this yet (these days we are preparing an internal release and that took away my time).

I have created a separate patch for the test related refactor, this patch now
depends on that patch and contains merely the visibility related change and
their tests.

• Remove dumpDeclContext() call
• Remove superfluous else

• Remove old style import in case of FoundArray

@shafik Ok, I'll take a look into the regression soon. What are exactly gmodules? Is it macOS only? Do you have a description or some docs about it?

Shafik, thanks for addressing the comments. This looks good to me now!

So the problem is that there are references to ParmVarDecl from inside function body and at import of ParmVarDecl always a new one is created even if there is an already existing (in the existing function prototype)?

Yes. During the import of the body we import a DeclRefExpr which refers to a ParmVarDecl but not the existing one, thus a new is created.

The updated version looks good to me! Thank you!

Shafik,
I have realized what's the problem with the ctu-main test. When we import the body and set the new body to the existing FunctionDecl then the parameters are still the old parameters so the new body does not refer to the formal parameters! This way the analyzer legally thinks that the parameter is not used inside the function because there is no DeclRef to that :(
This could be solved only if we merge *every* parts precisely, including the parameters, body, noexcept specifier, etc. But this would be a huge work.

I have found some other minor things in the tests.

@martong the only issue is that I am seeing a regression on Analysis/ctu-main.cpp when I run check-clang. I am going to look into it but if you have any insights that would be helpful.

Hi Shafik, thank you for this patch! Generally it looks quite okay to me, but I have a few minor comments.

Just a quick note. We are pretty sure that StructuralEquivalency can have false positive results, i.e. it can report two decls as nonequivalent falsely. My understanding is that we should report an error only if we are absolutely certain that an error has happened, this is not the case with StructuralEquivalency. Consequently this should be a warning in Sema (modules) too.

Looks good to me (but xazax's comments are valid)! Thank you.