On Tue, Nov 11, 2014 at 3:58 PM, Kaelyn Takata <[email protected]> wrote:
> > > On Tue, Nov 11, 2014 at 3:38 PM, David Blaikie <[email protected]> wrote: > >> >> >> On Tue, Nov 11, 2014 at 3:26 PM, Kaelyn Takata <[email protected]> wrote: >> >>> Author: rikka >>> Date: Tue Nov 11 17:26:56 2014 >>> New Revision: 221735 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=221735&view=rev >>> Log: >>> Create two helpers for running the typo-correction tree transform. >>> >>> One takes an Expr* and the other is a simple wrapper that takes an >>> ExprResult instead, and handles checking whether the ExprResult is >>> invalid. >>> >>> Additionally, allow an optional callback that is run on the full result >>> of the tree transform, for filtering potential corrections based on the >>> characteristics of the resulting expression once all of the typos have >>> been replaced. >>> >>> Modified: >>> cfe/trunk/include/clang/Sema/Sema.h >>> cfe/trunk/lib/Sema/SemaExprCXX.cpp >>> >>> Modified: cfe/trunk/include/clang/Sema/Sema.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=221735&r1=221734&r2=221735&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/include/clang/Sema/Sema.h (original) >>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Nov 11 17:26:56 2014 >>> @@ -2718,6 +2718,18 @@ public: >>> bool EnteringContext = false, >>> const ObjCObjectPointerType *OPT = >>> nullptr); >>> >>> + ExprResult >>> + CorrectDelayedTyposInExpr(Expr *E, >>> + llvm::function_ref<ExprResult(Expr *)> >>> Filter = >>> + [](Expr *E) -> ExprResult { return E; >>> }); >>> >> >> Not sure: is it worth pulling this trivial lambda out into a function & >> use it for the two sites here? (maybe "ExprResult::makeExprResult(Expr*)" >> ... though that seems almost silly ("make" functions feel like they should >> be doing template argument deduction, but that's just because that's their >> most common use)) >> >> (if you're keeping it as a lambda, you could write it as "[](Expr *E) { >> return ExprResult(E); }" if you think that's better >> > > Honestly, I wish I could make the default filter simply be the ExprResult > constructor, since that's pretty much what it is... > Yeah, if it were any static member/free function you could totally do this - just not the ctor... pity about that. > >> >>> + >>> + ExprResult >>> + CorrectDelayedTyposInExpr(ExprResult ER, >>> + llvm::function_ref<ExprResult(Expr *)> >>> Filter = >>> + [](Expr *E) -> ExprResult { return E; >>> }) { >>> + return ER.isInvalid() ? ER : CorrectDelayedTyposInExpr(ER.get(), >>> Filter); >>> + } >>> + >>> void diagnoseTypo(const TypoCorrection &Correction, >>> const PartialDiagnostic &TypoDiag, >>> bool ErrorRecovery = true); >>> >>> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=221735&r1=221734&r2=221735&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Nov 11 17:26:56 2014 >>> @@ -5919,6 +5919,7 @@ namespace { >>> class TransformTypos : public TreeTransform<TransformTypos> { >>> typedef TreeTransform<TransformTypos> BaseTransform; >>> >>> + llvm::function_ref<ExprResult(Expr *)> ExprFilter; >>> llvm::SmallSetVector<TypoExpr *, 2> TypoExprs; >>> llvm::SmallDenseMap<TypoExpr *, ExprResult, 2> TransformCache; >>> llvm::SmallDenseMap<OverloadExpr *, Expr *, 4> OverloadResolution; >>> @@ -5987,7 +5988,8 @@ class TransformTypos : public TreeTransf >>> } >>> >>> public: >>> - TransformTypos(Sema &SemaRef) : BaseTransform(SemaRef) {} >>> + TransformTypos(Sema &SemaRef, llvm::function_ref<ExprResult(Expr *)> >>> &&Filter) >>> >> >> Pass Filter by value if you're going to copy it anyway? >> > > Bizarre thing, I tried this when I originally updated this patch in > response to your earlier review feedback and for reasons unknown switching > this from passing by rvalue-reference to passing by value caused things to > break... yet just now I tried it (even with my latest in-progress changes) > and it behaved fine. > \o/... *frysquint* > >> >>> + : BaseTransform(SemaRef), ExprFilter(std::move(Filter)) {} >>> >>> ExprResult RebuildCallExpr(Expr *Callee, SourceLocation LParenLoc, >>> MultiExprArg Args, >>> @@ -6012,6 +6014,9 @@ public: >>> res = TransformExpr(E); >>> error = Trap.hasErrorOccurred(); >>> >>> + if (!(error || res.isInvalid())) >>> + res = ExprFilter(res.get()); >>> + >>> // Exit if either the transform was valid or if there were no >>> TypoExprs >>> // to transform that still have any untried correction >>> candidates.. >>> if (!(error || res.isInvalid()) || >>> @@ -6060,6 +6065,25 @@ public: >>> }; >>> } >>> >>> +ExprResult Sema::CorrectDelayedTyposInExpr( >>> + Expr *E, llvm::function_ref<ExprResult(Expr *)> Filter) { >>> + // If the current evaluation context indicates there are uncorrected >>> typos >>> + // and the current expression isn't guaranteed to not have typos, try >>> to >>> + // resolve any TypoExpr nodes that might be in the expression. >>> + if (!ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos && >>> + (E->isTypeDependent() || E->isValueDependent() || >>> + E->isInstantiationDependent())) { >>> + auto TyposResolved = DelayedTypos.size(); >>> + auto Result = TransformTypos(*this, std::move(Filter)).Transform(E); >>> + TyposResolved -= DelayedTypos.size(); >>> + if (TyposResolved) { >>> + ExprEvalContexts.back().NumTypos -= TyposResolved; >>> + return Result; >>> + } >>> + } >>> + return E; >>> +} >>> + >>> ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC, >>> bool DiscardedValue, >>> bool IsConstexpr, >>> @@ -6106,21 +6130,9 @@ ExprResult Sema::ActOnFinishFullExpr(Exp >>> return ExprError(); >>> } >>> >>> - // If the current evaluation context indicates there are uncorrected >>> typos >>> - // and the current expression isn't guaranteed to not have typos, try >>> to >>> - // resolve any TypoExpr nodes that might be in the expression. >>> - if (ExprEvalContexts.back().NumTypos && >>> - (FullExpr.get()->isTypeDependent() || >>> - FullExpr.get()->isValueDependent() || >>> - FullExpr.get()->isInstantiationDependent())) { >>> - auto TyposResolved = DelayedTypos.size(); >>> - FullExpr = TransformTypos(*this).Transform(FullExpr.get()); >>> - TyposResolved -= DelayedTypos.size(); >>> - if (TyposResolved) >>> - ExprEvalContexts.back().NumTypos -= TyposResolved; >>> - if (FullExpr.isInvalid()) >>> - return ExprError(); >>> - } >>> + FullExpr = CorrectDelayedTyposInExpr(FullExpr.get()); >>> + if (FullExpr.isInvalid()) >>> + return ExprError(); >>> >>> CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr); >>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
