[ASTImporter] Add importer specific lookup

[martong]

There are certain cases when normal C/C++ lookup (localUncachedLookup) does not find AST nodes. E.g.: Example 1:

  template <class T>
  struct X {
    friend void foo(); // this is never found in the DC of the TU.
  };

Example 2:

  // The fwd decl to Foo is not found in the lookupPtr of the DC of the
  // translation unit decl.
  struct A { struct Foo *p; };

In these cases we create a new node instead of returning with the old one. To fix it we create a new lookup table which holds every node and we are not interested in any C++ specific visibility considerations. Simply, we must know if there is an existing Decl in a given DC.

Fixes #503

[Xazax-hun]

Also, I think it would be great to add some high level documentation somewhere, why do we need a separate kind of lookup.

[martong]

Update: We discussed with @balazske that the cases when we compare an in-class method decl with an out-of-class method decl are signs of redecl chain errors. So I reverted the change related to in-class vs out-of-class check and fixed the redecl chain error rather, please see the commits.

[martong]

@Xazax-hun

Do we want to include everything in the huge lookup table? I am thinking about static declarations, and declarations in the anonymous namespace. We might not want to find those during our lookup. What do you think?

We have to find a previous static declaration from the same TU. Fortunately we already have a solution for that, we do filter the lookup results based on their visibility: See https://github.com/Ericsson/clang/pull/496

[martong]

Update: I have updated the PR, so it contains now 3 commits: 1) Add importer specific lookup 2) Set redecl chain of functions before any other import 3) Import overrides before importing the rest of the chain

2 and 3 are required in order to pass the Xerces analysis without assertions. They could go into a separate PR too , but then this PR and those too would not pass the CI. These are relatively very small changes, I've put them into separate commits to make it easier to upstream them independently.

[martong]

@balazske Ok, changed that and also changed the order of commits, so if there are any more comments about the new lookup i can change them easily.

[balazske]

I am interested in if the following code is still needed (this is in VisitRecordDecl and similar in VisitFunctionDecl). The tests show that the lookup result should contain both the templates and the templated decls (is this true in all kind of cases?) therefore this code could be omitted:

      if (D->getDescribedTemplate()) {
        if (auto *Template = dyn_cast<ClassTemplateDecl>(Found))
          Found = Template->getTemplatedDecl();
        else
          continue;
      }

[martong]

That is a good catch! So I changed it, but I was a bit worried that it may wreak havoc some lldb tests, because in case of lldb we still use the old C/C++ lookup. However, it looks like the lldb tests do pass, perhpas because they don't really import templates.

[martong]

@balazske Thanks for the review. I have squashed the last two commits into one.