[cfe-commits] r159980 - /cfe/trunk/docs/InternalsManual.html

[Jordan Rose]

This directly affects -Wobjc-literal-compare. In a private e-mail Chris suggested leaving the fixit on the warning itself (it was originally an error) since we're going to generate fairly useless code from what the user wrote 98% of the time. I (mildly) agree with Chandler about the meaning of fixits on warnings, though.

Currently -Wobjc-literal-compare actually DOES follow the fixit rule for ERRORS, that is, correct to use the fixit text even for CodeGen. This is not desirable behavior for this warning, but I don't know now whether to fix this by making it a note or just having Sema continue with the code as written. Chris implied the former is fine; Chandler says the latter would be preferred.

If we go with Chandler's interpretation (which is also what I remember from the discussion a few months ago) I'd still like to see the docs updated to point out that this needs to be very careful for warnings.

Seeking arbitration,
Jordan


On Jul 9, 2012, at 22:11 , Chandler Carruth <chandlerc at google.com> wrote:

> The decision we came to some time ago when discussing this on the mailing list was that the rule actually held, and the only valid fixit hint to issue on a warning is one which *does not impact* the AST produced or the recovery path.
> 
> For example, its valid to issue a warning with a fixit hint to correct 'struct' to 'class' (or vice versa) on a forward declaration which does not match the actual definition.
> 
> I think we have only a very few (if any) warnings left with fixit hints that, were they applied, would change the AST produced. And I think those should just be fixed to attach them to notes.
> 
> 
> On Mon, Jul 9, 2012 at 10:03 PM, Chris Lattner <sabre at nondot.org> wrote:
> Author: lattner
> Date: Tue Jul 10 00:03:05 2012
> New Revision: 159980
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=159980&view=rev
> Log:
> Jordan points out that this was incorrect: clang should recover from
> *errors* with fixits on them by following the recovery advised by the
> fixit, but if it is a fixit on a warning, then obviously the AST
> should be for the code as-written.
> 
> Modified:
>     cfe/trunk/docs/InternalsManual.html
> 
> Modified: cfe/trunk/docs/InternalsManual.html
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/InternalsManual.html?rev=159980&r1=159979&r2=159980&view=diff
> ==============================================================================
> --- cfe/trunk/docs/InternalsManual.html (original)
> +++ cfe/trunk/docs/InternalsManual.html Tue Jul 10 00:03:05 2012
> @@ -453,8 +453,7 @@
>  <li>Since they are automatically applied if <code>-Xclang -fixit</code>
>  is passed to the driver, they should only be used when it's very likely they
>  match the user's intent.</li>
> -<li>Clang must recover from the error or warning as if the fix-it had been
> -applied.</li>
> +<li>Clang must recover from errors as if the fix-it had been applied.</li>
>  </ul>
> 
>  <p>If a fix-it can't obey these rules, put the fix-it on a note. Fix-its on
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> _______________________________________________
> 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/20120713/79cf382f/attachment.html>