Skip to content

Commit 733edf9

Browse files
committed
[AST] Add RecoveryExpr to retain expressions on semantic errors
Normally clang avoids creating expressions when it encounters semantic errors, even if the parser knows which expression to produce. This works well for the compiler. However, this is not ideal for source-level tools that have to deal with broken code, e.g. clangd is not able to provide navigation features even for names that compiler knows how to resolve. The new RecoveryExpr aims to capture the minimal set of information useful for the tools that need to deal with incorrect code: source range of the expression being dropped, subexpressions of the expression. We aim to make constructing RecoveryExprs as simple as possible to ensure writing code to avoid dropping expressions is easy. Producing RecoveryExprs can result in new code paths being taken in the frontend. In particular, clang can produce some new diagnostics now and we aim to suppress bogus ones based on Expr::containsErrors. We deliberately produce RecoveryExprs only in the parser for now to minimize the code affected by this patch. Producing RecoveryExprs in Sema potentially allows to preserve more information (e.g. type of an expression), but also results in more code being affected. E.g. SFINAE checks will have to take presence of RecoveryExprs into account. Initial implementation only works in C++ mode, as it relies on compiler postponing diagnostics on dependent expressions. C and ObjC often do not do this, so they require more work to make sure we do not produce too many bogus diagnostics on the new expressions. See documentation of RecoveryExpr for more details. original patch from Ilya This change is based on https://reviews.llvm.org/D61722 Reviewers: sammccall, rsmith Reviewed By: sammccall, rsmith Tags: #clang Differential Revision: https://reviews.llvm.org/D69330
1 parent e0279d7 commit 733edf9

30 files changed

+424
-24
lines changed

clang/include/clang/AST/ComputeDependence.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class ExtVectorElementExpr;
4545
class BlockExpr;
4646
class AsTypeExpr;
4747
class DeclRefExpr;
48+
class RecoveryExpr;
4849
class CXXRewrittenBinaryOperator;
4950
class CXXStdInitializerListExpr;
5051
class CXXTypeidExpr;
@@ -122,6 +123,7 @@ ExprDependence computeDependence(ExtVectorElementExpr *E);
122123
ExprDependence computeDependence(BlockExpr *E);
123124
ExprDependence computeDependence(AsTypeExpr *E);
124125
ExprDependence computeDependence(DeclRefExpr *E, const ASTContext &Ctx);
126+
ExprDependence computeDependence(RecoveryExpr *E);
125127
ExprDependence computeDependence(CXXRewrittenBinaryOperator *E);
126128
ExprDependence computeDependence(CXXStdInitializerListExpr *E);
127129
ExprDependence computeDependence(CXXTypeidExpr *E);

clang/include/clang/AST/Expr.h

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5911,6 +5911,69 @@ class TypoExpr : public Expr {
59115911
}
59125912

59135913
};
5914+
5915+
/// Frontend produces RecoveryExprs on semantic errors that prevent creating
5916+
/// other well-formed expressions. E.g. when type-checking of a binary operator
5917+
/// fails, we cannot produce a BinaryOperator expression. Instead, we can choose
5918+
/// to produce a recovery expression storing left and right operands.
5919+
///
5920+
/// RecoveryExpr does not have any semantic meaning in C++, it is only useful to
5921+
/// preserve expressions in AST that would otherwise be dropped. It captures
5922+
/// subexpressions of some expression that we could not construct and source
5923+
/// range covered by the expression.
5924+
///
5925+
/// For now, RecoveryExpr is type-, value- and instantiation-dependent to take
5926+
/// advantage of existing machinery to deal with dependent code in C++, e.g.
5927+
/// RecoveryExpr is preserved in `decltype(<broken-expr>)` as part of the
5928+
/// `DependentDecltypeType`. In addition to that, clang does not report most
5929+
/// errors on dependent expressions, so we get rid of bogus errors for free.
5930+
/// However, note that unlike other dependent expressions, RecoveryExpr can be
5931+
/// produced in non-template contexts.
5932+
///
5933+
/// One can also reliably suppress all bogus errors on expressions containing
5934+
/// recovery expressions by examining results of Expr::containsErrors().
5935+
class RecoveryExpr final : public Expr,
5936+
private llvm::TrailingObjects<RecoveryExpr, Expr *> {
5937+
public:
5938+
static RecoveryExpr *Create(ASTContext &Ctx, SourceLocation BeginLoc,
5939+
SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
5940+
static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs);
5941+
5942+
ArrayRef<Expr *> subExpressions() {
5943+
auto *B = getTrailingObjects<Expr *>();
5944+
return llvm::makeArrayRef(B, B + NumExprs);
5945+
}
5946+
5947+
ArrayRef<const Expr *> subExpressions() const {
5948+
return const_cast<RecoveryExpr *>(this)->subExpressions();
5949+
}
5950+
5951+
child_range children() {
5952+
Stmt **B = reinterpret_cast<Stmt **>(getTrailingObjects<Expr *>());
5953+
return child_range(B, B + NumExprs);
5954+
}
5955+
5956+
SourceLocation getBeginLoc() const { return BeginLoc; }
5957+
SourceLocation getEndLoc() const { return EndLoc; }
5958+
5959+
static bool classof(const Stmt *T) {
5960+
return T->getStmtClass() == RecoveryExprClass;
5961+
}
5962+
5963+
private:
5964+
RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc, SourceLocation EndLoc,
5965+
ArrayRef<Expr *> SubExprs);
5966+
RecoveryExpr(EmptyShell Empty) : Expr(RecoveryExprClass, Empty) {}
5967+
5968+
size_t numTrailingObjects(OverloadToken<Stmt *>) const { return NumExprs; }
5969+
5970+
SourceLocation BeginLoc, EndLoc;
5971+
unsigned NumExprs;
5972+
friend TrailingObjects;
5973+
friend class ASTStmtReader;
5974+
friend class ASTStmtWriter;
5975+
};
5976+
59145977
} // end namespace clang
59155978

59165979
#endif // LLVM_CLANG_AST_EXPR_H

clang/include/clang/AST/RecursiveASTVisitor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2668,6 +2668,7 @@ DEF_TRAVERSE_STMT(CXXRewrittenBinaryOperator, {
26682668
})
26692669
DEF_TRAVERSE_STMT(OpaqueValueExpr, {})
26702670
DEF_TRAVERSE_STMT(TypoExpr, {})
2671+
DEF_TRAVERSE_STMT(RecoveryExpr, {})
26712672
DEF_TRAVERSE_STMT(CUDAKernelCallExpr, {})
26722673

26732674
// These operators (all of them) do not need any action except

clang/include/clang/Basic/LangOptions.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of template t
148148

149149
LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for all language standard modes")
150150

151+
COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "Preserve expressions in AST when encountering errors")
152+
151153
BENIGN_LANGOPT(ThreadsafeStatics , 1, 1, "thread-safe static initializers")
152154
LANGOPT(POSIXThreads , 1, 0, "POSIX thread support")
153155
LANGOPT(Blocks , 1, 0, "blocks extension to C")

clang/include/clang/Basic/StmtNodes.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ def ConvertVectorExpr : StmtNode<Expr>;
195195
def BlockExpr : StmtNode<Expr>;
196196
def OpaqueValueExpr : StmtNode<Expr>;
197197
def TypoExpr : StmtNode<Expr>;
198+
def RecoveryExpr : StmtNode<Expr>;
198199
def BuiltinBitCastExpr : StmtNode<ExplicitCastExpr>;
199200

200201
// Microsoft Extensions.

clang/include/clang/Driver/CC1Options.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,11 @@ def fno_concept_satisfaction_caching : Flag<["-"],
566566
"fno-concept-satisfaction-caching">,
567567
HelpText<"Disable satisfaction caching for C++2a Concepts.">;
568568

569+
def frecovery_ast : Flag<["-"], "frecovery-ast">,
570+
HelpText<"Preserve expressions in AST rather than dropping them when "
571+
"encountering semantic errors">;
572+
def fno_recovery_ast : Flag<["-"], "fno-recovery-ast">;
573+
569574
let Group = Action_Group in {
570575

571576
def Eonly : Flag<["-"], "Eonly">,

clang/include/clang/Sema/Sema.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3885,6 +3885,10 @@ class Sema final {
38853885
void DiagnoseAmbiguousLookup(LookupResult &Result);
38863886
//@}
38873887

3888+
/// Attempts to produce a RecoveryExpr after some AST node cannot be created.
3889+
ExprResult CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
3890+
ArrayRef<Expr *> SubExprs);
3891+
38883892
ObjCInterfaceDecl *getObjCInterfaceDecl(IdentifierInfo *&Id,
38893893
SourceLocation IdLoc,
38903894
bool TypoCorrection = false);

clang/include/clang/Serialization/ASTBitCodes.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,6 +1634,9 @@ namespace serialization {
16341634
/// An AtomicExpr record.
16351635
EXPR_ATOMIC,
16361636

1637+
/// A RecoveryExpr record.
1638+
EXPR_RECOVERY,
1639+
16371640
// Objective-C
16381641

16391642
/// An ObjCStringLiteral record.

clang/lib/AST/ComputeDependence.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,15 @@ ExprDependence clang::computeDependence(DeclRefExpr *E, const ASTContext &Ctx) {
456456
return Deps;
457457
}
458458

459+
ExprDependence clang::computeDependence(RecoveryExpr *E) {
460+
// FIXME: drop type+value+instantiation once Error is sufficient to suppress
461+
// bogus dianostics.
462+
auto D = ExprDependence::TypeValueInstantiation | ExprDependence::Error;
463+
for (auto *S : E->subExpressions())
464+
D |= S->getDependence();
465+
return D;
466+
}
467+
459468
ExprDependence clang::computeDependence(PredefinedExpr *E) {
460469
return toExprDependence(E->getType()->getDependence()) &
461470
~ExprDependence::UnexpandedPack;

clang/lib/AST/Expr.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2375,6 +2375,7 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
23752375
// If we don't know precisely what we're looking at, let's not warn.
23762376
case UnresolvedLookupExprClass:
23772377
case CXXUnresolvedConstructExprClass:
2378+
case RecoveryExprClass:
23782379
return false;
23792380

23802381
case CXXTemporaryObjectExprClass:
@@ -3227,6 +3228,7 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
32273228
case SubstNonTypeTemplateParmPackExprClass:
32283229
case FunctionParmPackExprClass:
32293230
case TypoExprClass:
3231+
case RecoveryExprClass:
32303232
case CXXFoldExprClass:
32313233
llvm_unreachable("shouldn't see dependent / unresolved nodes here");
32323234

@@ -4467,3 +4469,30 @@ QualType OMPArraySectionExpr::getBaseOriginalType(const Expr *Base) {
44674469
}
44684470
return OriginalTy;
44694471
}
4472+
4473+
RecoveryExpr::RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc,
4474+
SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
4475+
: Expr(RecoveryExprClass, Ctx.DependentTy, VK_LValue, OK_Ordinary),
4476+
BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) {
4477+
#ifndef NDEBUG
4478+
for (auto *E : SubExprs)
4479+
assert(E != nullptr);
4480+
#endif
4481+
4482+
llvm::copy(SubExprs, getTrailingObjects<Expr *>());
4483+
setDependence(computeDependence(this));
4484+
}
4485+
4486+
RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, SourceLocation BeginLoc,
4487+
SourceLocation EndLoc,
4488+
ArrayRef<Expr *> SubExprs) {
4489+
void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(SubExprs.size()),
4490+
alignof(RecoveryExpr));
4491+
return new (Mem) RecoveryExpr(Ctx, BeginLoc, EndLoc, SubExprs);
4492+
}
4493+
4494+
RecoveryExpr *RecoveryExpr::CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs) {
4495+
void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(NumSubExprs),
4496+
alignof(RecoveryExpr));
4497+
return new (Mem) RecoveryExpr(EmptyShell());
4498+
}

0 commit comments

Comments
 (0)