r221735 - Create two helpers for running the typo-correction tree transform.
Kaelyn Takata
rikka at google.com
Tue Nov 11 16:04:26 PST 2014
On Tue, Nov 11, 2014 at 4:00 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Tue, Nov 11, 2014 at 3:58 PM, Kaelyn Takata <rikka at google.com> wrote:
>
>>
>>
>> On Tue, Nov 11, 2014 at 3:38 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Nov 11, 2014 at 3:26 PM, Kaelyn Takata <rikka at google.com> 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
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141111/f5395a2e/attachment.html>
More information about the cfe-commits
mailing list