Import default expression of param before creating the param

[balazske]

Probably there are problems with this correction that can lead to infinite loop at import.

[martong]

Probably there are problems with this correction that can lead to infinite loop at import.

Could you please elaborate?

[balazske]

The problem happens when during the import of the default expression for a ParmVarDecl of a function the same function is imported again. Because it is not yet in AlreadyImportedDecls it is imported again. The import of a default expression can trigger the import of a DeclContext where a function exists that uses the original function with default argument, there is a CXXDefaultArgExpr. The "UsedContext" in it is set to the function that has the default argument. This is the thing that was hard to reproduce, and the error with infinite import and "stack overflow" (I did see multiple "random" errors in that case) did not happen again later (not the same source code but with the "bad" fix included). A test is needed when in the current test:

int DefParmContext::f() {
  return fDefParm(); // <-- the getUsedContext of CXXDefaultArgExpr should return FunctionDecl of fDefParm
}

[martong]

The problem happens when during the import of the default expression for a ParmVarDecl of a function the same function is imported again. Because it is not yet in AlreadyImportedDecls

How is it possible that an imported FunctionDecl is not yet in the AlreadyImportedDecls? We ought to put that there immediately after we created the node with FunctionDecl::Create. That's the point of the GetImportedOrCreateSpecialDecl()

[martong]

Perhaps a stack trace could explain better.

[balazske]

After this fix the default expression is imported before the function is created:

  } else if (D->hasDefaultArg()) {
    if (Error Err = importInto(DefaultArg, D->getDefaultArg()))
      return std::move(Err);
  }

  ParmVarDecl *ToParm;
  if (GetImportedOrCreateDecl(ToParm, D, Importer.getToContext(), DC,

And I did observe that the "used context" of a default arg expression can be the function from which the default arg expression is imported so this can cause infinite import loop:

ExpectedStmt ASTNodeImporter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
  ExpectedSLoc ToUsedLocOrErr = import(E->getUsedLocation());
  if (!ToUsedLocOrErr)
    return ToUsedLocOrErr.takeError();

  auto ToParamOrErr = import(E->getParam());
  if (!ToParamOrErr)
    return ToParamOrErr.takeError();

  auto UsedContextOrErr = Importer.ImportContext(E->getUsedContext());
  if (!UsedContextOrErr)
    return UsedContextOrErr.takeError();

(The CXXDefaultArgExpr appears when the function is called with the default argument used. This call must be encountered during the import of the default expression above to get the problem. This condition is done in the test but not the condition that the getUsedContext returns the needed function.)

With the new fix (https://reviews.llvm.org/D65577) the import of the default expression is done after create of the function so it is included in AlreadyImportedDecls.

[martong]

Okay, thanks for the explanation!