martong added inline comments.
================
Comment at: lib/AST/ASTImporter.cpp:4967
+template <typename T> static auto getTemplateDefinition(T *D) -> T * {
+ auto *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
if (!ToTemplatedDef)
----------------
shafik wrote:
> shafik wrote:
> > Can we guarantee that `D->getTemplatedDecl()` will always return a valid
> > pointer?
> What other types besides `CXXRecordDecl` do we expect here?
> Can we guarantee that D->getTemplatedDecl() will always return a valid
> pointer?
Actually, we always call this function on a `ClassTemplateDecl` or on a
`FunctionTemplateDecl` which should return with a valid pointer. Still it is a
good idea to put an assert at the beginning of the function, so I just did that.
```
assert(D->getTemplatedDecl() && "Should be called on templates only");
```
> What other types besides CXXRecordDecl do we expect here?
`FunctionDecl` can be there too when we call this on a `FunctiomTemplateDecl`
param.
================
Comment at: lib/AST/ASTImporter.cpp:5544
// type, and in the same context as the function we're importing.
+ // FIXME Split this into a separate function.
if (!LexicalDC->isFunctionOrMethod()) {
----------------
shafik wrote:
> Would it make sense to do the split into a separate function in the PR?
I'd rather do that in another patch where we could address the similar issues
in other visit functions too. Actually, most of the visit functions start with
a lookup and then continue with a loop over the lookup results; this part could
be split into a separate function everywhere.
================
Comment at: lib/AST/ASTImporter.cpp:5595
+ auto *PrevTemplated =
+ FoundByLookup->getTemplatedDecl()->getMostRecentDecl();
+ if (TemplatedFD != PrevTemplated)
----------------
shafik wrote:
> Can we guarantee that `FoundByLookup->getTemplatedDecl()` will always return
> a valid pointer?
This is a good catch, thanks.
The `FoundByLookup` variable's type is `FunctionTemplateDecl`. Either the found
Decl is created by the parser then it must have set up a templated decl, or it
is created by the ASTImporter and we should have a templated decl set.
In the second case, however, we may not be 100% sure that the templated is
really set.
So I have added an assert here too.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58494/new/
https://reviews.llvm.org/D58494
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
