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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 276700.
hokein marked 4 inline comments as done.
hokein added a comment.

refine the doc based on the review comments.


Repository:
  rG LLVM Github Monorepo

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

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
@@ -19210,9 +19210,6 @@
 
 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();
 
Index: clang/lib/AST/ComputeDependence.cpp
===================================================================
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -495,11 +495,21 @@
 }
 
 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())
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.
-/// 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:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83215.276700.patch
Type: text/x-patch
Size: 4401 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200709/dab9a453/attachment.bin>