Skip to content

Remove delayed typo expressions #143423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jun 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/unittests/HoverTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ class Foo final {})cpp";
HI.Name = "abc";
HI.Kind = index::SymbolKind::Variable;
HI.NamespaceScope = "";
HI.Definition = "int abc = <recovery - expr>()";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

despite typoexprs fragile and complicated nature, i think they were providing most of their value by preserving more nodes in the AST, that can later on be used by source tools (especially for more error correction :)).

I don't think we need TypoExprs for most of this, if possible, preserving these TypoExpr nodes through RecoveryExprs would actually help a lot with preserving current behavior of tools but also shape of the AST for any other application.

I wouldn't block on this though, and wouldn't be surprised if this isn't trivial, we don't seem to be preserving typoexprs as recovery-exprs most of the time and I am not sure why.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, making a RecoveryExpr in these cases would be helpful (though potentially nontrivial). There was another case where we dropped a ReturnStmt rather than giving it a RecoveryExpr operand, and that caused follow-on diagnostics that were less than helpful.

HI.Definition = "int abc";
HI.Type = "int";
HI.AccessSpecifier = "public";
}},
Expand Down
8 changes: 8 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,14 @@ Improvements to Clang's diagnostics

- Improved the FixIts for unused lambda captures.

- Delayed typo correction was removed from the compiler; immediate typo
correction behavior remains the same. Delayed typo correction facilities were
fragile and unmaintained, and the removal closed the following issues:
#GH142457, #GH139913, #GH138850, #GH137867, #GH137860, #GH107840, #GH93308,
#GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490,
#GH36703, #GH32903, #GH23312, #GH69874.


Improvements to Clang's time-trace
----------------------------------

Expand Down
33 changes: 1 addition & 32 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,7 @@ class Expr : public ValueStmt {
return static_cast<bool>(getDependence() & ExprDependence::UnexpandedPack);
}

/// Whether this expression contains subexpressions which had errors, e.g. a
/// TypoExpr.
/// Whether this expression contains subexpressions which had errors.
bool containsErrors() const {
return static_cast<bool>(getDependence() & ExprDependence::Error);
}
Expand Down Expand Up @@ -6965,36 +6964,6 @@ class AtomicExpr : public Expr {
}
};

/// TypoExpr - Internal placeholder for expressions where typo correction
/// still needs to be performed and/or an error diagnostic emitted.
class TypoExpr : public Expr {
// The location for the typo name.
SourceLocation TypoLoc;

public:
TypoExpr(QualType T, SourceLocation TypoLoc)
: Expr(TypoExprClass, T, VK_LValue, OK_Ordinary), TypoLoc(TypoLoc) {
assert(T->isDependentType() && "TypoExpr given a non-dependent type");
setDependence(ExprDependence::TypeValueInstantiation |
ExprDependence::Error);
}

child_range children() {
return child_range(child_iterator(), child_iterator());
}
const_child_range children() const {
return const_child_range(const_child_iterator(), const_child_iterator());
}

SourceLocation getBeginLoc() const LLVM_READONLY { return TypoLoc; }
SourceLocation getEndLoc() const LLVM_READONLY { return TypoLoc; }

static bool classof(const Stmt *T) {
return T->getStmtClass() == TypoExprClass;
}

};

/// This class represents BOTH the OpenMP Array Section and OpenACC 'subarray',
/// with a boolean differentiator.
/// OpenMP 5.0 [2.1.5, Array Sections].
Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/AST/RecursiveASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2956,7 +2956,6 @@ DEF_TRAVERSE_STMT(CXXRewrittenBinaryOperator, {
}
})
DEF_TRAVERSE_STMT(OpaqueValueExpr, {})
DEF_TRAVERSE_STMT(TypoExpr, {})
DEF_TRAVERSE_STMT(RecoveryExpr, {})
DEF_TRAVERSE_STMT(CUDAKernelCallExpr, {})

Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/Basic/StmtNodes.td
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ def ShuffleVectorExpr : StmtNode<Expr>;
def ConvertVectorExpr : StmtNode<Expr>;
def BlockExpr : StmtNode<Expr>;
def OpaqueValueExpr : StmtNode<Expr>;
def TypoExpr : StmtNode<Expr>;
def RecoveryExpr : StmtNode<Expr>;
def BuiltinBitCastExpr : StmtNode<ExplicitCastExpr>;
def EmbedExpr : StmtNode<Expr>;
Expand Down
3 changes: 1 addition & 2 deletions clang/include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -4171,8 +4171,7 @@ class Parser : public CodeCompletionHandler {
bool ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
llvm::function_ref<void()> ExpressionStarts =
llvm::function_ref<void()>(),
bool FailImmediatelyOnInvalidExpr = false,
bool EarlyTypoCorrection = false);
bool FailImmediatelyOnInvalidExpr = false);

/// ParseSimpleExpressionList - A simple comma-separated list of expressions,
/// used for misc language extensions.
Expand Down
126 changes: 5 additions & 121 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -6713,10 +6713,6 @@ class Sema final : public SemaBase {
/// this expression evaluation context.
unsigned NumCleanupObjects;

/// The number of typos encountered during this expression evaluation
/// context (i.e. the number of TypoExprs created).
unsigned NumTypos;

MaybeODRUseExprSet SavedMaybeODRUseExprs;

/// The lambdas that are present within this context, if it
Expand Down Expand Up @@ -6813,7 +6809,7 @@ class Sema final : public SemaBase {
Decl *ManglingContextDecl,
ExpressionKind ExprContext)
: Context(Context), ParentCleanup(ParentCleanup),
NumCleanupObjects(NumCleanupObjects), NumTypos(0),
NumCleanupObjects(NumCleanupObjects),
ManglingContextDecl(ManglingContextDecl), ExprContext(ExprContext),
InDiscardedStatement(false), InImmediateFunctionContext(false),
InImmediateEscalatingFunctionContext(false) {}
Expand Down Expand Up @@ -7146,8 +7142,7 @@ class Sema final : public SemaBase {
CorrectionCandidateCallback &CCC,
TemplateArgumentListInfo *ExplicitTemplateArgs = nullptr,
ArrayRef<Expr *> Args = {},
DeclContext *LookupCtx = nullptr,
TypoExpr **Out = nullptr);
DeclContext *LookupCtx = nullptr);

/// If \p D cannot be odr-used in the current expression evaluation context,
/// return a reason explaining why. Otherwise, return NOUR_None.
Expand Down Expand Up @@ -8748,40 +8743,6 @@ class Sema final : public SemaBase {

ExprResult CheckUnevaluatedOperand(Expr *E);

/// Process any TypoExprs in the given Expr and its children,
/// generating diagnostics as appropriate and returning a new Expr if there
/// were typos that were all successfully corrected and ExprError if one or
/// more typos could not be corrected.
///
/// \param E The Expr to check for TypoExprs.
///
/// \param InitDecl A VarDecl to avoid because the Expr being corrected is its
/// initializer.
///
/// \param RecoverUncorrectedTypos If true, when typo correction fails, it
/// will rebuild the given Expr with all TypoExprs degraded to RecoveryExprs.
///
/// \param Filter A function applied to a newly rebuilt Expr to determine if
/// it is an acceptable/usable result from a single combination of typo
/// corrections. As long as the filter returns ExprError, different
/// combinations of corrections will be tried until all are exhausted.
ExprResult CorrectDelayedTyposInExpr(
Expr *E, VarDecl *InitDecl = nullptr,
bool RecoverUncorrectedTypos = false,
llvm::function_ref<ExprResult(Expr *)> Filter =
[](Expr *E) -> ExprResult { return E; });

ExprResult CorrectDelayedTyposInExpr(
ExprResult ER, VarDecl *InitDecl = nullptr,
bool RecoverUncorrectedTypos = false,
llvm::function_ref<ExprResult(Expr *)> Filter =
[](Expr *E) -> ExprResult { return E; }) {
return ER.isInvalid()
? ER
: CorrectDelayedTyposInExpr(ER.get(), InitDecl,
RecoverUncorrectedTypos, Filter);
}

IfExistsResult
CheckMicrosoftIfExistsSymbol(Scope *S, CXXScopeSpec &SS,
const DeclarationNameInfo &TargetNameInfo);
Expand Down Expand Up @@ -9283,12 +9244,6 @@ class Sema final : public SemaBase {
/// for C++ records.
llvm::FoldingSet<SpecialMemberOverloadResultEntry> SpecialMemberCache;

/// Holds TypoExprs that are created from `createDelayedTypo`. This is used by
/// `TransformTypos` in order to keep track of any TypoExprs that are created
/// recursively during typo correction and wipe them away if the correction
/// fails.
llvm::SmallVector<TypoExpr *, 2> TypoExprs;

enum class AcceptableKind { Visible, Reachable };

// Members have to be NamespaceDecl* or TranslationUnitDecl*.
Expand Down Expand Up @@ -9376,10 +9331,6 @@ class Sema final : public SemaBase {
bool VolatileArg, bool RValueThis, bool ConstThis,
bool VolatileThis);

typedef std::function<void(const TypoCorrection &)> TypoDiagnosticGenerator;
typedef std::function<ExprResult(Sema &, TypoExpr *, TypoCorrection)>
TypoRecoveryCallback;

RedeclarationKind forRedeclarationInCurContext() const;

/// Look up a name, looking for a single declaration. Return
Expand Down Expand Up @@ -9733,51 +9684,6 @@ class Sema final : public SemaBase {
const ObjCObjectPointerType *OPT = nullptr,
bool RecordFailure = true);

/// Try to "correct" a typo in the source code by finding
/// visible declarations whose names are similar to the name that was
/// present in the source code.
///
/// \param TypoName the \c DeclarationNameInfo structure that contains
/// the name that was present in the source code along with its location.
///
/// \param LookupKind the name-lookup criteria used to search for the name.
///
/// \param S the scope in which name lookup occurs.
///
/// \param SS the nested-name-specifier that precedes the name we're
/// looking for, if present.
///
/// \param CCC A CorrectionCandidateCallback object that provides further
/// validation of typo correction candidates. It also provides flags for
/// determining the set of keywords permitted.
///
/// \param TDG A TypoDiagnosticGenerator functor that will be used to print
/// diagnostics when the actual typo correction is attempted.
///
/// \param TRC A TypoRecoveryCallback functor that will be used to build an
/// Expr from a typo correction candidate.
///
/// \param MemberContext if non-NULL, the context in which to look for
/// a member access expression.
///
/// \param EnteringContext whether we're entering the context described by
/// the nested-name-specifier SS.
///
/// \param OPT when non-NULL, the search for visible declarations will
/// also walk the protocols in the qualified interfaces of \p OPT.
///
/// \returns a new \c TypoExpr that will later be replaced in the AST with an
/// Expr representing the result of performing typo correction, or nullptr if
/// typo correction is not possible. If nullptr is returned, no diagnostics
/// will be emitted and it is the responsibility of the caller to emit any
/// that are needed.
TypoExpr *CorrectTypoDelayed(
const DeclarationNameInfo &Typo, Sema::LookupNameKind LookupKind,
Scope *S, CXXScopeSpec *SS, CorrectionCandidateCallback &CCC,
TypoDiagnosticGenerator TDG, TypoRecoveryCallback TRC,
CorrectTypoKind Mode, DeclContext *MemberContext = nullptr,
bool EnteringContext = false, const ObjCObjectPointerType *OPT = nullptr);

/// Kinds of missing import. Note, the values of these enumerators correspond
/// to %select values in diagnostics.
enum class MissingImportKind {
Expand All @@ -9796,20 +9702,6 @@ class Sema final : public SemaBase {
SourceLocation DeclLoc, ArrayRef<Module *> Modules,
MissingImportKind MIK, bool Recover);

struct TypoExprState {
std::unique_ptr<TypoCorrectionConsumer> Consumer;
TypoDiagnosticGenerator DiagHandler;
TypoRecoveryCallback RecoveryHandler;
TypoExprState();
TypoExprState(TypoExprState &&other) noexcept;
TypoExprState &operator=(TypoExprState &&other) noexcept;
};

const TypoExprState &getTypoExprState(TypoExpr *TE) const;

/// Clears the state of the given TypoExpr.
void clearDelayedTypo(TypoExpr *TE);

/// Called on #pragma clang __debug dump II
void ActOnPragmaDump(Scope *S, SourceLocation Loc, IdentifierInfo *II);

Expand All @@ -9832,23 +9724,15 @@ class Sema final : public SemaBase {
/// Determine if we could use all the declarations in the module.
bool isUsableModule(const Module *M);

/// Helper for CorrectTypo and CorrectTypoDelayed used to create and
/// populate a new TypoCorrectionConsumer. Returns nullptr if typo correction
/// should be skipped entirely.
/// Helper for CorrectTypo used to create and populate a new
/// TypoCorrectionConsumer. Returns nullptr if typo correction should be
/// skipped entirely.
std::unique_ptr<TypoCorrectionConsumer> makeTypoCorrectionConsumer(
const DeclarationNameInfo &Typo, Sema::LookupNameKind LookupKind,
Scope *S, CXXScopeSpec *SS, CorrectionCandidateCallback &CCC,
DeclContext *MemberContext, bool EnteringContext,
const ObjCObjectPointerType *OPT, bool ErrorRecovery);

/// The set of unhandled TypoExprs and their associated state.
llvm::MapVector<TypoExpr *, TypoExprState> DelayedTypos;

/// Creates a new TypoExpr AST node.
TypoExpr *createDelayedTypo(std::unique_ptr<TypoCorrectionConsumer> TCC,
TypoDiagnosticGenerator TDG,
TypoRecoveryCallback TRC, SourceLocation TypoLoc);

/// Cache for module units which is usable for current module.
llvm::DenseSet<const Module *> UsableModuleUnitsCache;

Expand Down
14 changes: 0 additions & 14 deletions clang/include/clang/Sema/SemaInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,20 +314,6 @@ class TypoCorrectionConsumer : public VisibleDeclConsumer {
bool SearchNamespaces;
};

inline Sema::TypoExprState::TypoExprState() {}

inline Sema::TypoExprState::TypoExprState(TypoExprState &&other) noexcept {
*this = std::move(other);
}

inline Sema::TypoExprState &Sema::TypoExprState::
operator=(Sema::TypoExprState &&other) noexcept {
Consumer = std::move(other.Consumer);
DiagHandler = std::move(other.DiagHandler);
RecoveryHandler = std::move(other.RecoveryHandler);
return *this;
}

} // end namespace clang

#endif
1 change: 0 additions & 1 deletion clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3611,7 +3611,6 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
case PackExpansionExprClass:
case SubstNonTypeTemplateParmPackExprClass:
case FunctionParmPackExprClass:
case TypoExprClass:
case RecoveryExprClass:
case CXXFoldExprClass:
// Make a conservative assumption for dependent nodes.
Expand Down
1 change: 0 additions & 1 deletion clang/lib/AST/ExprClassification.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
// FIXME: Is this wise? Should they get their own kind?
case Expr::UnresolvedLookupExprClass:
case Expr::UnresolvedMemberExprClass:
case Expr::TypoExprClass:
case Expr::DependentCoawaitExprClass:
case Expr::CXXDependentScopeMemberExprClass:
case Expr::DependentScopeDeclRefExprClass:
Expand Down
1 change: 0 additions & 1 deletion clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17327,7 +17327,6 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
case Expr::CXXDeleteExprClass:
case Expr::CXXPseudoDestructorExprClass:
case Expr::UnresolvedLookupExprClass:
case Expr::TypoExprClass:
case Expr::RecoveryExprClass:
case Expr::DependentScopeDeclRefExprClass:
case Expr::CXXConstructExprClass:
Expand Down
1 change: 0 additions & 1 deletion clang/lib/AST/ItaniumMangle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4994,7 +4994,6 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
case Expr::ParenListExprClass:
case Expr::MSPropertyRefExprClass:
case Expr::MSPropertySubscriptExprClass:
case Expr::TypoExprClass: // This should no longer exist in the AST by now.
case Expr::RecoveryExprClass:
case Expr::ArraySectionExprClass:
case Expr::OMPArrayShapingExprClass:
Expand Down
5 changes: 0 additions & 5 deletions clang/lib/AST/StmtPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2914,11 +2914,6 @@ void StmtPrinter::VisitOpaqueValueExpr(OpaqueValueExpr *Node) {
PrintExpr(Node->getSourceExpr());
}

void StmtPrinter::VisitTypoExpr(TypoExpr *Node) {
// TODO: Print something reasonable for a TypoExpr, if necessary.
llvm_unreachable("Cannot print TypoExpr nodes");
}

void StmtPrinter::VisitRecoveryExpr(RecoveryExpr *Node) {
OS << "<recovery-expr>(";
const char *Sep = "";
Expand Down
4 changes: 0 additions & 4 deletions clang/lib/AST/StmtProfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2361,10 +2361,6 @@ void StmtProfiler::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
VisitExpr(E);
}

void StmtProfiler::VisitTypoExpr(const TypoExpr *E) {
VisitExpr(E);
}

void StmtProfiler::VisitSourceLocExpr(const SourceLocExpr *E) {
VisitExpr(E);
}
Expand Down
1 change: 0 additions & 1 deletion clang/lib/Parse/ParseCXXInlineMethods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,6 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) {
DefArgResult = ParseBraceInitializer();
} else
DefArgResult = ParseAssignmentExpression();
DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
if (DefArgResult.isInvalid()) {
Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
/*DefaultArg=*/nullptr);
Expand Down
Loading
Loading