[PATCH] D16478: Always build a new TypeSourceInfo for function templates with parameters

[Nico Weber via cfe-commits]

thakis added a comment.

In http://reviews.llvm.org/D16478#338653, @rnk wrote:

> The fact that an instantiated type might point to decls from the template pattern seems like expected behavior. The parts of the template that are the same are supposed to be shared.
>
> Can we dig in to what is going wrong in parent map construction?


"RecursiveASTVisitor::TraverseFunctionHelper() traverses a function's ParmVarDecls by going to the function's getTypeSourceInfo if it exists, and `
DEF_TRAVERSE_TYPELOC(FunctionProtoType` then goes to the function's ParmVarDecls." was my attempt to do that. I guess I can expand this a bit with code snippets.

The parent map uses a RecursiveASTVisitor to build parents: It keeps a stack of nodes it has seen so far, and when something gets visited, it marks parent[something] = seenstack.top(). RecursiveASTVisitor does this for functions:

  DEF_TRAVERSE_DECL(FunctionDecl, {
    // We skip decls_begin/decls_end, which are already covered by
    // TraverseFunctionHelper().
    return TraverseFunctionHelper(D);
  })

TraverseFunctionHelper goes to the TypeSource if it exists to visit the parameter decls:

  bool RecursiveASTVisitor<Derived>::TraverseFunctionHelper(FunctionDecl *D) {
    // ...
    if (TypeSourceInfo *TSI = D->getTypeSourceInfo()) {
      TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));

The TypeLoc then goes to the param's decl if available:

  DEF_TRAVERSE_TYPELOC(FunctionProtoType, {
    TRY_TO(TraverseTypeLoc(TL.getReturnLoc()));
  
    const FunctionProtoType *T = TL.getTypePtr();
  
    for (unsigned I = 0, E = TL.getNumParams(); I != E; ++I) {
      if (TL.getParam(I)) {

So when the instantiated CXXMethodDecl for quantizedSize is visited, this goes and visits the ParmDecls from the template CXXMethodDecl, not from the instantiated one. Hence, the instantiated ParmDecl for count never gets a parent assigned (since it's never visited).

So at least RecursiveASTVisitor didn't expect the current behavior.


http://reviews.llvm.org/D16478