[PATCH] D83213: [clang][RecoveryExpr] Clarify the dependence-bits documentation.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/lib/AST/ComputeDependence.cpp:502
+  //     dependent type), or the type is known and dependent, or it has
+  //     type-dependent subexpressions
   auto D = toExprDependence(E->getType()->getDependence()) |
----------------
sammccall wrote:
> hmm, I'd missed the type-dependent subexpressions question.
> If there are type-dependent subexpressions, but a non-dependent type was specified for the RecoveryExpr, is the expr type-dependent?
> 
> This is the part that I think we have discretion over.
> The definition of type-dependent does say "any type-dependent subexpression" but then lays out a list of exceptions such as casts, which are not type-dependent even if their argument is. What these have in common is that the type is known.
> 
> So I think this comes down to whether it's the caller's job to work this out, or we want to conservatively call these expressions dependent.
> 
> I think the former is probably better - marking the expression as type-dependent but not having its actual type be dependent doesn't serve any purpose I'm aware of. It's also inconsistent with the informal definition of type-dependence described earlier in this patch.
> 
> So the comment should describe the current state, but maybe a FIXME to remove the type-dependent subexpressions clause?
yeah, I think we should respect to actual type of RecoveryExpr, added a fixme.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83213