martong added a comment.
> We can't reliable import templates with the ASTImporter, so actually
> reconstructing the template in the debug info AST and then importing it
> doesn't work.
Could you please elaborate how the import of templates fails in ASTImporter? Is
it because the AST you
balazske added a comment.
By the way if the `Import` of the strategy uses recursive import of other
things this can cause same problems as it was in `ASTImporter` before the
`GetImportedOrCreateDecl` was introduced. So this should be avoided or
something similar to `GetImportedOrCreateDecl`
balazske added a comment.
Another observation: The `Import` function of the strategy now has no way to
return an error. An even better version of it would be to include the
possibility of import error (with `ImportError`, or other error type). Or the
"PreImport" function could indicate if the
teemperor planned changes to this revision.
teemperor added a comment.
I think we could also refactor the strategy into a `PreImport` call. That way
we don't break all the user-code out there and it seems less intrusive to the
ASTImporter code. I'll update the patch.
CHANGES SINCE LAST ACTION
balazske added a comment.
It looks better now.
One "problem" is that now there is the strategy and there is the `Imported`
function. It would be possible to unify these into a strategy that contains
maybe a "import" and a "post-import" callback. (But this requires big changes
in the
teemperor updated this revision to Diff 191257.
teemperor retitled this revision from "[ASTImporter] Allow adding a shim to the
ASTImporter" to "[ASTImporter] Allow adding a import strategy to the
ASTImporter".
teemperor edited the summary of this revision.
teemperor added a comment.
Thanks for
