mizvekov added inline comments.

================
Comment at: clang/lib/AST/ASTContext.cpp:12767
+        Ctx.getQualifiedType(Underlying),
+        ::getCommonDecl(EX->getOwnedTagDecl(), EY->getOwnedTagDecl()));
+  }
----------------
davrec wrote:
> This last argument should probably be omitted/nullptr passed, since
> # We probably won't encounter any owned tag decls in the types passed to this 
> function;
> # Even if we did, it would not necessarily be true that they both have 
> non-null owned tag decls, and I don't see any nullptr checks in 
> getCommonDecl; and
> # Even if we checked for nullptr there, and say passed the same argument to X 
> and Y so EX==EY, and that had an owned tag decl, it is not clear to me it 
> would be appropriate to construct a new type with the same owned tag decl as 
> another type.
1. We can see, a one liner like this will do: `int e = true ? (struct S*)0 : 
(struct S*)0;`

Though normally in an example like the above, you would only see the owned decl 
on X, because when we get to Y the name has already been introduced into the 
scope.

I have an idea for a test case, but it's a bit convoluted:

This also introduces an OwnedTagDecl: `(struct S<T>*)0`.

So with resugaring, if we resugar T, it might be possible to construct this 
situatiation. Given if it's appropriate to keep the OwnedTagDecl when 
resugaring, of course, which goes to 3)

2. This is handled by `declaresSameEntity`, if either or both decls are null, 
we say they are not the same decl.

3. That I am not sure. I see that this OwnedTagDecl is only used by the type 
printer, and it replaces printing the rest of the type by printing the 
OwnedDecl instead. So why do you think it would not be appropriate that the 
rebuilt type keeps this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130308/new/

https://reviews.llvm.org/D130308

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

Reply via email to