r191544 - Avoid the hard-coded limit on the number of typo corrections attempted.

Kaelyn Uhrain rikka at google.com
Tue Oct 1 14:45:15 PDT 2013


On Tue, Oct 1, 2013 at 2:34 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Fri, Sep 27, 2013 at 12:40 PM, Kaelyn Uhrain <rikka at google.com> wrote:
>>
>> Author: rikka
>> Date: Fri Sep 27 14:40:12 2013
>> New Revision: 191544
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=191544&view=rev
>> Log:
>> Avoid the hard-coded limit on the number of typo corrections attempted.
>>
>> Move some tests from typo-correction.cpp to typo-correction-pt2.cpp
>> because they were running afoul of the hard-coded limit of 20 typos
>> corrected. Some of the tests after it were still working due to the
>> limit not applying to cached corrections and in cases where a non-NULL
>> MemberContext is passed in to Sema::CorrectTypo.  Most of the moved tests
>> still passed after being moved, but the test involving "data_struct" had
>> only been passing because the test had exceeded that limit so a fix for
>> it is also included (most of the changes to ParseStmt.cpp are shared with
>> and originated from another typo correction impovement that was split
>> into a separate commit).
>>
>> Modified:
>>     cfe/trunk/lib/Parse/ParseStmt.cpp
>>     cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp
>>     cfe/trunk/test/SemaCXX/typo-correction.cpp
>>
>> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=191544&r1=191543&r2=191544&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Fri Sep 27 14:40:12 2013
>> @@ -111,6 +111,33 @@ Parser::ParseStatementOrDeclaration(Stmt
>>    return Actions.ProcessStmtAttributes(Res.get(), Attrs.getList(),
>> Attrs.Range);
>>  }
>>
>> +namespace {
>> +class StatementFilterCCC : public CorrectionCandidateCallback {
>> +public:
>> +  StatementFilterCCC(Token nextTok) : NextToken(nextTok) {
>
>
> Please name local variables starting with an uppercase letter in new code.
>
>>
>> +    WantTypeSpecifiers = nextTok.is(tok::l_paren) ||
>> nextTok.is(tok::less) ||
>> +                         nextTok.is(tok::identifier) ||
>> nextTok.is(tok::star) ||
>> +                         nextTok.is(tok::amp) ||
>> nextTok.is(tok::l_square);
>> +    WantExpressionKeywords = nextTok.is(tok::l_paren) ||
>> +                             nextTok.is(tok::identifier) ||
>> +                             nextTok.is(tok::arrow) ||
>> nextTok.is(tok::period);
>> +    WantRemainingKeywords = nextTok.is(tok::l_paren) ||
>> nextTok.is(tok::semi) ||
>> +                            nextTok.is(tok::identifier) ||
>> +                            nextTok.is(tok::l_brace);
>> +    WantCXXNamedCasts = false;
>> +  }
>> +
>> +  virtual bool ValidateCandidate(const TypoCorrection &candidate) {
>> +    if (FieldDecl *FD = candidate.getCorrectionDeclAs<FieldDecl>())
>> +      return isa<ObjCIvarDecl>(FD);
>
>
> Hmm, what happens if I write:
>
> struct S {
>   int my_member;
>   void f() { my_menber = 1; }
> };
>
> Will we still correct that?

Apparently not, and none of the existing tests tried to. :(

Thanks for pointing that case out! I hope to have a fix shortly.

>
>>
>> +    return CorrectionCandidateCallback::ValidateCandidate(candidate);
>> +  }
>> +
>> +private:
>> +  Token NextToken;
>> +};
>> +}
>> +
>>  StmtResult
>>  Parser::ParseStatementOrDeclarationAfterAttributes(StmtVector &Stmts,
>>            bool OnlyStatement, SourceLocation *TrailingElseLoc,
>> @@ -149,21 +176,8 @@ Retry:
>>      if (Next.isNot(tok::coloncolon)) {
>>        // Try to limit which sets of keywords should be included in typo
>>        // correction based on what the next token is.
>> -      // FIXME: Pass the next token into the CorrectionCandidateCallback
>> and
>> -      //        do this filtering in a more fine-grained manner.
>> -      CorrectionCandidateCallback DefaultValidator;
>> -      DefaultValidator.WantTypeSpecifiers =
>> -          Next.is(tok::l_paren) || Next.is(tok::less) ||
>> -          Next.is(tok::identifier) || Next.is(tok::star) ||
>> -          Next.is(tok::amp) || Next.is(tok::l_square);
>> -      DefaultValidator.WantExpressionKeywords =
>> -          Next.is(tok::l_paren) || Next.is(tok::identifier) ||
>> -          Next.is(tok::arrow) || Next.is(tok::period);
>> -      DefaultValidator.WantRemainingKeywords =
>> -          Next.is(tok::l_paren) || Next.is(tok::semi) ||
>> -          Next.is(tok::identifier) || Next.is(tok::l_brace);
>> -      DefaultValidator.WantCXXNamedCasts = false;
>> -      if (TryAnnotateName(/*IsAddressOfOperand*/false, &DefaultValidator)
>> +      StatementFilterCCC Validator(Next);
>> +      if (TryAnnotateName(/*IsAddressOfOperand*/false, &Validator)
>>              == ANK_Error) {
>>          // Handle errors here by skipping up to the next semicolon or
>> '}', and
>>          // eat the semicolon if that's what stopped us.
>>
>> Modified: cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp?rev=191544&r1=191543&r2=191544&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp Fri Sep 27 14:40:12
>> 2013
>> @@ -5,6 +5,37 @@
>>  // attempt within a single file (which is to avoid having very broken
>> files take
>>  // minutes to finally be rejected by the parser).
>>
>> +namespace bogus_keyword_suggestion {
>> +void test() {
>> +   status = "OK";  // expected-error-re {{use of undeclared identifier
>> 'status'$}}
>> +   return status;  // expected-error-re {{use of undeclared identifier
>> 'status'$}}
>> + }
>> +}
>> +
>> +namespace PR13387 {
>> +struct A {
>> +  void CreateFoo(float, float);
>> +  void CreateBar(float, float);
>> +};
>> +struct B : A {
>> +  using A::CreateFoo;
>> +  void CreateFoo(int, int);
>> +};
>> +void f(B &x) {
>> +  x.Createfoo(0,0);  // expected-error {{no member named 'Createfoo' in
>> 'PR13387::B'; did you mean 'CreateFoo'?}}
>> +}
>> +}
>> +
>> +struct DataStruct {void foo();};
>> +struct T {
>> + DataStruct data_struct;
>> + void f();
>> +};
>> +// should be void T::f();
>> +void f() {
>> + data_struct->foo();  // expected-error-re{{use of undeclared identifier
>> 'data_struct'$}}
>> +}
>> +
>>  namespace PR12287 {
>>  class zif {
>>    void nab(int);
>>
>> Modified: cfe/trunk/test/SemaCXX/typo-correction.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction.cpp?rev=191544&r1=191543&r2=191544&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/typo-correction.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/typo-correction.cpp Fri Sep 27 14:40:12 2013
>> @@ -246,37 +246,6 @@ namespace outer {
>>    }
>>  }
>>
>> -namespace bogus_keyword_suggestion {
>> -void test() {
>> -   status = "OK"; // expected-error-re{{use of undeclared identifier
>> 'status'$}}
>> -   return status; // expected-error-re{{use of undeclared identifier
>> 'status'$}}
>> - }
>> -}
>> -
>> -namespace PR13387 {
>> -struct A {
>> -  void CreateFoo(float, float);
>> -  void CreateBar(float, float);
>> -};
>> -struct B : A {
>> -  using A::CreateFoo;
>> -  void CreateFoo(int, int);
>> -};
>> -void f(B &x) {
>> -  x.Createfoo(0,0); // expected-error {{no member named 'Createfoo' in
>> 'PR13387::B'; did you mean 'CreateFoo'?}}
>> -}
>> -}
>> -
>> -struct DataStruct {void foo();};
>> -struct T {
>> - DataStruct data_struct;
>> - void f();
>> -};
>> -// should be void T::f();
>> -void f() {
>> - data_struct->foo(); // expected-error-re{{use of undeclared identifier
>> 'data_struct'$}}
>> -}
>> -
>>  namespace b6956809_test1 {
>>    struct A {};
>>    struct B {};
>> @@ -319,9 +288,13 @@ namespace b6956809_test2 {
>>    }
>>  }
>>
>> +// This test should have one correction, followed by an error without a
>> +// suggestion due to exceeding the maximum number of typos for which
>> correction
>> +// is attempted.
>>  namespace CorrectTypo_has_reached_its_limit {
>> -int flibberdy();  // no note here
>> +int flibberdy();  // expected-note{{'flibberdy' declared here}}
>>  int no_correction() {
>> -  return gibberdy();  // expected-error-re{{use of undeclared identifier
>> 'gibberdy'$}}
>> +  return hibberdy() +  // expected-error{{use of undeclared identifier
>> 'hibberdy'; did you mean 'flibberdy'?}}
>> +         gibberdy();  // expected-error-re{{use of undeclared identifier
>> 'gibberdy'$}}
>>  };
>>  }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>



More information about the cfe-commits mailing list