rsmith added a comment.

I think the idea of this change is OK. The key is that the dereference 
expression will still be type-dependent, even if we happen to actually know its 
type. (For an `Expr` that `isTypeDependent()`, the type produced by `getType()` 
isn't something the standard knows or cares about and is only used for our own 
type-checking-of-templates purposes.) The property we need to guarantee is 
that, if the `Expr` is valid, then the type is correct, and I think that holds 
here. However, I wouldn't want to guarantee that all parts of Clang get this 
right, and we might find some lurking bugs are exposed by this change.

I think this change is being made in an imperfect place, though. Instead of 
putting a special case in here, how would we feel about making 
`Type::isOverloadableType()` smarter, to return `false` for dependent types 
that can't possibly be class or enum types (eg, dependent pointer types, array 
types, and maybe more exotic things like function types, atomic types, and 
complex types)? This would then apply to all operators, not only unary `*`. Eg, 
we know the type of `&p` where `p` is `T*`, and we know the type of `p + n` 
where  `p` is `T*` and  `n` is integral. That change might have a lot more 
fallout, but it'd certainly be interesting to see what it breaks.

Is this change observable in some way? We should include a test with this 
change that demonstrates what effect it has. (The existing test in this patch 
passes with or without the change.) If nothing else I think we should be able 
to see the effect of this change in the output of `-ast-dump`. (Making the more 
general change I mentioned above may help here, by giving earlier diagnostics 
for some cases for which instantiation could never succeed, such as rejecting 
`p + q` where `p` and `q` are both pointer types.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112453

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

Reply via email to