[PATCH] D83215: [AST][RecoveryExpr] Clarify the documentation of RecoveryExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83215

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19211,9 +19211,6 @@
 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();
 
Index: clang/lib/AST/ComputeDependence.cpp
===================================================================
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -495,11 +495,19 @@
 }
 
 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())
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -6212,19 +6212,22 @@
 /// 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:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83215.275673.patch
Type: text/x-patch
Size: 4036 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200706/bceb0d7f/attachment-0001.bin>