[AST][RecoveryExpr] Clarify the documentation of RecoveryExpr.

[hokein]

Summary

Fix some out-of-date doc/FIXMEs, and be clear about our plan.

Depends on https://reviews.llvm.org/D83213.

[sammccall]

clang/include/clang/AST/Expr.h

6229

this could be a little more specific (e.g. what does C++ only mean)...
FIXME: RecoveryExpr is currently generated by default in C++ mode only, as dependence isn't handled properly on several C-only codepaths.

clang/lib/AST/ComputeDependence.cpp

498–512

I can't really follow this explanation.

• The first bullet says when, the other two bullets say why
• the reasons given don't seem to be very principled ones (e.g. suppressing constant-evaluation is indeed a nice effect of value-dependency, but I don't think it's the *reason* for the value-dependency, rather that the value depends on how the error is resolved)
• I don't understand the connection between the "setting" list and the "explanations" one.

499

, or if the type is known and dependent

[hokein]

refine the doc based on the review comments.

clang/lib/AST/ComputeDependence.cpp

498–512

sorry for the confusion. I have refined the doc, hope it is clearer now.

[hokein]

closing this in favor of D83213.

clang/include/clang/AST/Expr.h

 /// fails, we cannot produce a BinaryOperator expression. Instead, we can choose
 /// to produce a recovery expression storing left and right operands.
 ///
 /// RecoveryExpr does not have any semantic meaning in C++, it is only useful to
 /// preserve expressions in AST that would otherwise be dropped. It captures
 /// subexpressions of some expression that we could not construct and source
 /// range covered by the expression.
 ///
-/// By default, RecoveryExpr is type-, value- and instantiation-dependent to
-/// take advantage of existing machinery to deal with dependent code in C++,
-/// e.g. RecoveryExpr is preserved in `decltype(<broken-expr>)` as part of the
-/// `DependentDecltypeType`. In addition to that, clang does not report most
-/// errors on dependent expressions, so we get rid of bogus errors for free.
-/// However, note that unlike other dependent expressions, RecoveryExpr can be
-/// produced in non-template contexts.
-/// In addition, we will preserve the type in RecoveryExpr when the type is
-/// known, e.g. preserving the return type for a broken non-overloaded function
-/// call, a overloaded call where all candidates have the same return type.
+/// By default, RecoveryExpr uses dependence-bits to take advantage of existing
+/// machinery to deal with dependent code in C++, e.g. RecoveryExpr is preserved
+/// in `decltype(<broken-expr>)` as part of the `DependentDecltypeType`. In
+/// addition to that, clang does not report most errors on dependent
+/// expressions, so we get rid of bogus errors for free. However, note that
+/// unlike other dependent expressions, RecoveryExpr can be produced in
+/// non-template contexts. In addition, we will preserve the type in
+/// RecoveryExpr when the type is known, e.g. preserving the return type for a
+/// broken non-overloaded function call, a overloaded call where all candidates
+/// have the same return type.
 ///
 /// One can also reliably suppress all bogus errors on expressions containing
 /// recovery expressions by examining results of Expr::containsErrors().
+///
+/// FIXME: RecoveryExpr is currently generated by default in C++ mode only, as
+/// dependence isn't handled properly on several C-only codepaths.
 class RecoveryExpr final : public Expr,
                            private llvm::TrailingObjects<RecoveryExpr, Expr *> {
 public:
   static RecoveryExpr *Create(ASTContext &Ctx, QualType T,
                               SourceLocation BeginLoc, SourceLocation EndLoc,
                               ArrayRef<Expr *> SubExprs);
   static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs);
 

clang/lib/AST/ComputeDependence.cpp

ExprDependence clang::computeDependence(DeclRefExpr *E, const ASTContext &Ctx) {
   //      -  a member function or a static data member of the current
   //         instantiation
   if (isa<CXXMethodDecl>(Decl) && Decl->getDeclContext()->isDependentContext())
     Deps |= ExprDependence::ValueInstantiation;
   return Deps;
 }
 
 ExprDependence clang::computeDependence(RecoveryExpr *E) {
-  // Mark the expression as value- and instantiation- dependent to reuse
-  // existing suppressions for dependent code, e.g. avoiding
-  // constant-evaluation.
-  // FIXME: drop type+value+instantiation once Error is sufficient to suppress
-  // bogus dianostics.
+  // A dependent expression (typically in template context) can behave
+  // differently from one instantiation to another. The dependence-bits describe
+  // how an expression depends on a template parameter.
+  //
+  // We generalize the "dependent" to mean "depends on a template parameter, or
+  // an error".
+  //
+  // RecoveryExpression is
+  //  - always contains-errors: the expression depends on an error;
+  //  - always value-dependent: we never know the value, the value can be
+  //    different or unknown depending on how the error is resolved;
+  //  - always instantiation-dependent: historically, this bit is used to
+  //    determine whether an expression is dependent;
+  //  - type dependent if we don't know the type (fallback to an opequa
+  //    dependent type), or the type is known and dependent;
   auto D = toExprDependence(E->getType()->getDependence()) |
            ExprDependence::ValueInstantiation | ExprDependence::Error;
   for (auto *S : E->subExpressions())
     D |= S->getDependence();
   return D;
 }
 
 ExprDependence clang::computeDependence(PredefinedExpr *E) {

clang/lib/Sema/SemaExpr.cpp

else if (getCurBlock() || getCurLambda())
     getCurFunction()->HasPotentialAvailabilityViolations = true;
 
   return new (Context)
       ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }
 
 ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
                                     ArrayRef<Expr *> SubExprs, QualType T) {
-  // FIXME: enable it for C++, RecoveryExpr is type-dependent to suppress
-  // bogus diagnostics and this trick does not work in C.
-  // FIXME: use containsErrors() to suppress unwanted diags in C.
   if (!Context.getLangOpts().RecoveryAST)
     return ExprError();
 
   if (isSFINAEContext())
     return ExprError();
 
   if (T.isNull() || !Context.getLangOpts().RecoveryASTType)
     // We don't know the concrete type, fallback to dependent type.
     T = Context.DependentTy;
   return RecoveryExpr::Create(Context, T, Begin, End, SubExprs);
 }