<div class="gmail_quote">On Fri, Jun 29, 2012 at 2:36 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Fri, Jun 29, 2012 at 2:30 PM, Kaelyn Uhrain <<a href="mailto:rikka@google.com">rikka@google.com</a>> wrote:<br>
> Author: rikka<br>
> Date: Fri Jun 29 16:30:39 2012<br>
> New Revision: 159464<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=159464&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=159464&view=rev</a><br>
> Log:<br>
> In Sema::ClassifyName, try to avoid nonsensical corrections to<br>
> keywords when doing type correction.<br>
><br>
> Modified:<br>
>    cfe/trunk/lib/Sema/SemaDecl.cpp<br>
>    cfe/trunk/test/SemaCXX/typo-correction.cpp<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=159464&r1=159463&r2=159464&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=159464&r1=159463&r2=159464&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jun 29 16:30:39 2012<br>
> @@ -634,6 +634,19 @@<br>
>     if (!SecondTry) {<br>
>       SecondTry = true;<br>
>       CorrectionCandidateCallback DefaultValidator;<br>
> +      // Try to limit which sets of keywords should be included in typo<br>
> +      // correction based on what the next token is.<br>
> +      DefaultValidator.WantTypeSpecifiers =<br>
> +          NextToken.is(tok::l_paren) || NextToken.is(tok::less) ||<br>
> +          NextToken.is(tok::identifier) || NextToken.is(tok::star) ||<br>
> +          NextToken.is(tok::amp) || NextToken.is(tok::l_square);<br>
> +      DefaultValidator.WantExpressionKeywords =<br>
> +          NextToken.is(tok::l_paren) || NextToken.is(tok::identifier) ||<br>
> +          NextToken.is(tok::arrow) || NextToken.is(tok::period);<br>
> +      DefaultValidator.WantRemainingKeywords =<br>
> +          NextToken.is(tok::l_paren) || NextToken.is(tok::semi) ||<br>
> +          NextToken.is(tok::identifier) || NextToken.is(tok::l_brace);<br>
> +      DefaultValidator.WantCXXNamedCasts = false;<br>
>       if (TypoCorrection Corrected = CorrectTypo(Result.getLookupNameInfo(),<br>
>                                                  Result.getLookupKind(), S,<br>
>                                                  &SS, DefaultValidator)) {<br>
><br>
> Modified: cfe/trunk/test/SemaCXX/typo-correction.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction.cpp?rev=159464&r1=159463&r2=159464&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction.cpp?rev=159464&r1=159463&r2=159464&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/typo-correction.cpp (original)<br>
> +++ cfe/trunk/test/SemaCXX/typo-correction.cpp Fri Jun 29 16:30:39 2012<br>
> @@ -227,3 +227,10 @@<br>
>  // 'boo' to 'bool' is the same edit distance as correcting 'boo' to 'foo'.<br>
>  class bar : boo { }; // expected-error{{unknown class name 'boo'; did you mean 'foo'?}}<br>
>  }<br>
> +<br>
> +namespace bogus_keyword_suggestion {<br>
> +void test() {<br>
<br>
</div></div>Would it make sense to have this test case have a reasonable return<br>
type for the code/return statement in its body? Or you can leave it as<br>
is & someone else can change it if the diagnostic/recovery gets<br>
further improvements in this area.<br></blockquote><div><br></div><div>Ah, yes... the test case is copied straight from the internal bug that Nick filed, so I missed that. I'm torn, but I think it might be better to leave it as is as a test case related to future diagnostic improvements in this area... such as if that 'status = "OK";' were to be recovered as 'auto status = "OK";' as an instance where that recovery would more likely be incorrect, i.e. the function was refactored to no longer return the status and that assignment and return statement were missed, or if they were part of a block of code copied and pasted from a function that did return a value. ;)  But I can go either way with whether to change the function return type to be something sensible.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> +   status = "OK"; // expected-error-re{{use of undeclared identifier 'status'$}}<br>
<br>
</div>Would it be completely silly to recover as if the user wrote "auto x =<br>
y;" whenever we see "x = y;" with x being undeclared? (we don't<br>
necessarily have to suggest it as a fixit, but might make for a better<br>
recovery than assumed int or not adding this to the symbol table at<br>
all)<br>
<span class="HOEnZb"><font color="#888888"><br>
- David<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> +   return status; // expected-error-re{{use of undeclared identifier 'status'$}}<br>
> + }<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br>