r221735 - Create two helpers for running the typo-correction tree transform.

[David Blaikie]

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*


>
>>
>>> +      : 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/74598de3/attachment.html>