[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

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

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.
+/// 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 C++ only, make it work for C by supporting dependence
+/// mechanism for C language in clang.
 class RecoveryExpr final : public Expr,
                            private llvm::TrailingObjects<RecoveryExpr, Expr *> {
 public:
   static RecoveryExpr *Create(ASTContext &Ctx, QualType T, ExprValueKind VK,
                               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.
+  // RecoveryExpr dependence-bits setting:
+  //   - type-dep is set if we don't know about the type (fallback to an opaque
+  //     dependent type);
+  //   - value-dep is always set to avoid constant-evaluation;
+  //   - instantiation-dep bit is not set to disinguish with regular template
+  //     dependent expressions;
+  //
+  // Explanations:
+  //   - "type-dependent" + "contains-errors": depends on an error and we don't
+  //     know the type
+  //   - "contains-errors": depends on an error and we know the type
+  //   - "type-depdendent" + "instantiation-dependent": a regular dependent
+  //     expression which involves template parameters (not a recovery expr)
   auto D = toExprDependence(E->getType()->getDependence()) |
            ExprDependence::Value | ExprDependence::Error;
   for (auto *S : E->subExpressions())
     D |= S->getDependence();
   return D;
 }
 
 ExprDependence clang::computeDependence(PredefinedExpr *E) {

clang/lib/Sema/SemaExpr.cpp

ExprResult Sema::ActOnObjCAvailabilityCheckExpr(
 
   return new (Context)
       ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }
 
 ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
                                     ArrayRef<Expr *> SubExprs, QualType T,
                                     ExprValueKind VK) {
-  // 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;
     VK = VK_LValue;
   }
   return RecoveryExpr::Create(Context, T, VK, Begin, End, SubExprs);
 }