[llvm-branch-commits] [clang] 3c1fca8 - Fix issue in typo handling which could lead clang to hang
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jul 27 01:43:11 PDT 2020
I think this patch is okay, but in general please check with me before
pushing to the release branch.
Thanks,
Hans
On Fri, Jul 24, 2020 at 11:35 PM David Goldman via llvm-branch-commits
<llvm-branch-commits at lists.llvm.org> wrote:
>
>
> Author: David Goldman
> Date: 2020-07-24T17:02:06-04:00
> New Revision: 3c1fca803bc14617b67ba2125e1b4b77190e9f86
>
> URL: https://github.com/llvm/llvm-project/commit/3c1fca803bc14617b67ba2125e1b4b77190e9f86
> DIFF: https://github.com/llvm/llvm-project/commit/3c1fca803bc14617b67ba2125e1b4b77190e9f86.diff
>
> LOG: Fix issue in typo handling which could lead clang to hang
>
> Summary:
> We need to detect when certain TypoExprs are not being transformed
> due to invalid trees, otherwise we risk endlessly trying to fix it.
>
> Reviewers: rsmith
>
> Subscribers: cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D84067
>
> (cherry picked from commit dde98c82c0ad02410229e7e5c9efcbb0ab42a995)
>
> Added:
> clang/test/Sema/typo-correction-no-hang.cpp
>
> Modified:
> clang/include/clang/Sema/SemaInternal.h
> clang/lib/Sema/SemaExprCXX.cpp
> clang/test/Sema/typo-correction-recursive.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/clang/include/clang/Sema/SemaInternal.h b/clang/include/clang/Sema/SemaInternal.h
> index cdaf7b70a92f..842eec099540 100644
> --- a/clang/include/clang/Sema/SemaInternal.h
> +++ b/clang/include/clang/Sema/SemaInternal.h
> @@ -168,6 +168,11 @@ class TypoCorrectionConsumer : public VisibleDeclConsumer {
> return TC;
> }
>
> + /// In the case of deeply invalid expressions, `getNextCorrection()` will
> + /// never be called since the transform never makes progress. If we don't
> + /// detect this we risk trying to correct typos forever.
> + bool hasMadeAnyCorrectionProgress() const { return CurrentTCIndex != 0; }
> +
> /// Reset the consumer's position in the stream of viable corrections
> /// (i.e. getNextCorrection() will return each of the previously returned
> /// corrections in order before returning any new corrections).
>
> diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
> index d885920b6c14..77bd1ab360b2 100644
> --- a/clang/lib/Sema/SemaExprCXX.cpp
> +++ b/clang/lib/Sema/SemaExprCXX.cpp
> @@ -7977,19 +7977,26 @@ class TransformTypos : public TreeTransform<TransformTypos> {
> }
> }
>
> - /// If corrections for the first TypoExpr have been exhausted for a
> - /// given combination of the other TypoExprs, retry those corrections against
> - /// the next combination of substitutions for the other TypoExprs by advancing
> - /// to the next potential correction of the second TypoExpr. For the second
> - /// and subsequent TypoExprs, if its stream of corrections has been exhausted,
> - /// the stream is reset and the next TypoExpr's stream is advanced by one (a
> - /// TypoExpr's correction stream is advanced by removing the TypoExpr from the
> - /// TransformCache). Returns true if there is still any untried combinations
> - /// of corrections.
> + /// Try to advance the typo correction state of the first unfinished TypoExpr.
> + /// We allow advancement of the correction stream by removing it from the
> + /// TransformCache which allows `TransformTypoExpr` to advance during the
> + /// next transformation attempt.
> + ///
> + /// Any substitution attempts for the previous TypoExprs (which must have been
> + /// finished) will need to be retried since it's possible that they will now
> + /// be invalid given the latest advancement.
> + ///
> + /// We need to be sure that we're making progress - it's possible that the
> + /// tree is so malformed that the transform never makes it to the
> + /// `TransformTypoExpr`.
> + ///
> + /// Returns true if there are any untried correction combinations.
> bool CheckAndAdvanceTypoExprCorrectionStreams() {
> for (auto TE : TypoExprs) {
> auto &State = SemaRef.getTypoExprState(TE);
> TransformCache.erase(TE);
> + if (!State.Consumer->hasMadeAnyCorrectionProgress())
> + return false;
> if (!State.Consumer->finished())
> return true;
> State.Consumer->resetCorrectionStream();
>
> diff --git a/clang/test/Sema/typo-correction-no-hang.cpp b/clang/test/Sema/typo-correction-no-hang.cpp
> new file mode 100644
> index 000000000000..3c591645be25
> --- /dev/null
> +++ b/clang/test/Sema/typo-correction-no-hang.cpp
> @@ -0,0 +1,40 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +// From `test/Sema/typo-correction.c` but for C++ since the behavior varies
> +// between the two languages.
> +struct rdar38642201 {
> + int fieldName;
> +};
> +
> +void rdar38642201_callee(int x, int y);
> +void rdar38642201_caller() {
> + struct rdar38642201 structVar;
> + rdar38642201_callee(
> + structVar1.fieldName1.member1, //expected-error{{use of undeclared identifier 'structVar1'}}
> + structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}}
> +}
> +
> +// Similar reproducer.
> +class A {
> +public:
> + int minut() const = delete;
> + int hour() const = delete;
> +
> + int longit() const; //expected-note{{'longit' declared here}}
> + int latit() const;
> +};
> +
> +class B {
> +public:
> + A depar() const { return A(); }
> +};
> +
> +int Foo(const B &b) {
> + return b.deparT().hours() * 60 + //expected-error{{no member named 'deparT' in 'B'}}
> + b.deparT().minutes(); //expected-error{{no member named 'deparT' in 'B'}}
> +}
> +
> +int Bar(const B &b) {
> + return b.depar().longitude() + //expected-error{{no member named 'longitude' in 'A'; did you mean 'longit'?}}
> + b.depar().latitude(); //expected-error{{no member named 'latitude' in 'A'}}
> +}
>
> diff --git a/clang/test/Sema/typo-correction-recursive.cpp b/clang/test/Sema/typo-correction-recursive.cpp
> index 48bd3b80c599..b39beb5493f6 100644
> --- a/clang/test/Sema/typo-correction-recursive.cpp
> +++ b/clang/test/Sema/typo-correction-recursive.cpp
> @@ -118,3 +118,15 @@ int testDeepAmbiguity() {
> asDeepASItGet().
> functionE();
> }
> +
> +struct Dog {
> + int age; //expected-note{{'age' declared here}}
> + int size; //expected-note{{'size' declared here}}
> +};
> +
> +int from_dog_years(int DogYears, int DogSize);
> +int get_dog_years() {
> + struct Dog doggo;
> + return from_dog_years(doggo.agee, //expected-error{{no member named 'agee' in 'Dog'; did you mean 'age'}}
> + doggo.sizee); //expected-error{{no member named 'sizee' in 'Dog'; did you mean 'size'}}
> +}
>
>
>
> _______________________________________________
> llvm-branch-commits mailing list
> llvm-branch-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
More information about the llvm-branch-commits
mailing list