martong added inline comments.
Herald added a reviewer: shafik.
Herald added a subscriber: gamesh411.


================
Comment at: lib/AST/ASTImporter.cpp:2310
+                D->getUnderlyingType(), FoundTypedef->getUnderlyingType())) {
+          QualType FromUT = D->getUnderlyingType();
+          QualType FoundUT = FoundTypedef->getUnderlyingType();
----------------
a_sidorin wrote:
> We can move these two vars upper and use them in the condition.
Good point, changed it.


================
Comment at: lib/AST/ASTImporter.cpp:2314
+          // already have a complete underlying type then return with that.
+          if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
             return Importer.MapImported(D, FoundTypedef);
----------------
a_sidorin wrote:
> This condition omits the case where both types are complete. Should we use 
> `FromUT->isIncompleteType == FoundUT-isIncompleteType()` instead?
I think you wanted to say "both types are INcomplete".
The condition indeed omits the case where both types are incomplete and that is 
a potential bug, thanks for finding it!
I think the best way to handle this case is to do the something similar what is 
done in case of functions, i.e break the for loop once we know that the found 
and the to be imported Decl are structurally equivalent.
So, I added a `break` to do that and to be similar what we do in case of 
functions.

This is what we have with functions:
```
          if (IsStructuralMatch(D, FoundFunction)) {
            const FunctionDecl *Definition = nullptr;
            if (D->doesThisDeclarationHaveABody() &&
                FoundFunction->hasBody(Definition)) {
              return Importer.MapImported(
                  D, const_cast<FunctionDecl *>(Definition));
            }
            FoundByLookup = FoundFunction;
            break;
          }
```

I'd like to have something similar with typedefs. The goal is to have only one 
TypedefNameDecl with a complete type. A TypedefNameDecl with a complete type is 
analogous to a FunctionDecl with a definition, we want to have only one in the 
"To" context.
A TypedefNameDecl is also redeclarable, so on a long term we should connect a 
new Decl to the found previous one.




Repository:
  rC Clang

https://reviews.llvm.org/D53693



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to