[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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