[cfe-dev] Alternative FixItHints

[Richard Smith via cfe-dev]

On Wed, Jun 1, 2016 at 11:46 AM, Manuel Klimek via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> On Wed, Jun 1, 2016 at 6:59 PM David Blaikie via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> I think the general idea in Clang has been that fixits on a warning must
>> be behavior-preserving
>>
>
> Typo-correction is not behavior preserving, though?
>

We do not issue typo-correction FixItHints on warnings. (Well, actually, I
think we have a single warning that does this, but it's a bug.)

(because we must recover from errors (and warnings can be errors) as if the
>> fixit were applied, so that downstream diagnostics/fixits are applicable).
>> We use notes to express other fixit alternatives (or any non-behavior
>> preserving fixits if there's no behavior-preserving fixit we care to give).
>> Should we not be using notes as alternatives in the include fixer tool?
>>
>
+1

This is our usual approach for the case where there are multiple possible
fixes: one note per possible approach, with the FixItHints for that
approach attached to that note. Is there some reason why this doesn't apply
in your case?


> Is there some semantic information missing from multiple notes with fixits
>> that we could improve instead? (that would maintain/improve the existing
>> command line usability) to accurately describe which notes form the set of
>> alternative solutions? (so that an IDE, etc, could collapse/render them
>> differently, perhaps?)
>>
>> On Wed, May 25, 2016 at 8:24 AM, Benjamin Kramer via cfe-dev <
>> cfe-dev at lists.llvm.org> wrote:
>>
>>> Hi all,
>>>
>>> one of the longer-term goals for the include fixer tool is integration
>>> into the editor. This could piggy-back on the existing support for
>>> FixIts but there is one big missing feature. It is often not perfectly
>>> clear what #include a user wants or if it's just a typo that needs
>>> correcting, we cannot express that with the current FixIts. A Clang
>>> diagnostic can have multiple FixIts but they are meant to be applied
>>> together, for this case we'd need alternative groups of FixIts.
>>>
>>> There's also a fair number of existing clang warnings that could
>>> benefit from this, typo correction is an obvious one but there's also
>>> a bunch of disambiguation warnings that can have fix-its in both
>>> directions, for example '= in if statement' (turn into '==' or add
>>> double parens) or '&& within ||' where parens can be added both ways.
>>> The functionality-preserving FixIt is the canonical one for those
>>> warnings but having an alternative would be useful, too.
>>>
>>> Of course this won't work on the terminal. FixIts are already hardly
>>> usable there, so this will be a feature for IDEs, code review tools
>>> etc.
>>>
>>> My current plan is:
>>>   1. Have DiagnosticEngine and friends keep a vector of vector of
>>> FixItHint instead of a flat FixIt vector
>>>   2. Thread the change through clang, using alternative '0' in most
>>> places to avoid breaking existing functionality.
>>>   3. Add methods to add alternative FixItHints to a diagnostic
>>>   4. Expose this via a new libclang API. Any diagnostic rendering code
>>> in clang will stay the same.
>>>
>>> There's interest from YouCompleteMe for providing an interface for
>>> this that lets the user pick a fix out of multiple ones, and I guess
>>> other IDEs can put it to great use too.
>>>
>>> Any input on this approach or concerns?
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160601/90173d06/attachment.html>