On Tue, Nov 11, 2014 at 4:00 PM, David Blaikie <[email protected]> wrote:
> > > 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* > I need to amend my statement. It isn't the rvalue-ref vs value that breaks things... the magic is actually in the std::move--after sending that email I realized I tested the removal of "&&" but didn't change "ExprFilter(std::move(Filter))" to simply "ExprFilter(Filter)". Once I did that on a client without any other local changed, 7 unit tests had segfaults. > >> >>> >>>> + : 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
