On Tue, Nov 11, 2014 at 4:04 PM, Kaelyn Takata <[email protected]> wrote:
> > > 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. > Subtle bug in function_ref is subtle. Fixed in LLVM in r221753 - you should now be able to just pass the function_refs around by value, drop the std::move - function_ref is so cheap it's silly to explicitly move it around. Richard - let me know how that LLVM change looks (CC'ing you here just so you have the extra context to our earlier discussion). > > >> >>> >>>> >>>>> + : 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
