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 <[email protected]> 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 > [email protected] > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
