[cfe-dev] Alternative FixItHints

Mon Jun 20 08:29:04 PDT 2016

On Mon, Jun 20, 2016 at 1:24 AM Ben Jackson <puremourning at gmail.com> wrote:

> Just following up on this thread: Yes it totally makes sense.
>
> Previously we were ignoring any fixit hints attached to “child”
> diagnostics (in libclang parlance). I’ve made a quick change to YCM which:
>  - extracts fixit hints from chid diagnostics (typically notes)
>  - provides a UI in Vim to select between them when multiple fixits are
> available for a given diag.
>
> It works nicely for the case you mention below:
> https://files.gitter.im/Valloric/YouCompleteMe/l6FD/YCM-fixit-multiple-choice2.gif.
> Incidentally, this also improves the workflow when there are multiple
> diagnostics on the line.
>

This is awesome \o/ Thanks!

> With regard to Manuel’s question about who should be responsible for
> choosing the insertion location for, say includes, I’d be keen for clang to
> make that decision, as it has better knowledge about code than the IDE
> (consider objective-c vs. c vs…). It also requires less work from each
> client implementation.
>

I guess the work for us for the future is that we'll need to somehow expose
a way to format code replacements, but that'll need to be designed first.

> That said, in YCM we already have some stuff for working out where to put
> automatically generated import declarations for c-sharp, and we even
> provide hooks for the user to tune to their taste, so there is an argument
> that where the decision is arbitrary, the client should offer the user more
> flexibility.
>
> On 2 Jun 2016, at 12:00, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
>
> Ben, would it make sense for YCM to provide the user with choice when
> there are notes attached to a diagnostic? You can see this in action
> for -Wparentheses:
>
> $ cat t.c
> void f(int foo, int bar) {
>  if (foo = bar) {}
> }
>
> $ clang t.c
> t.c:2:11: warning: using the result of an assignment as a condition without
>      parentheses [-Wparentheses]
>  if (foo = bar) {}
>      ~~~~^~~~~
> t.c:2:11: note: place parentheses around the assignment to silence this
>      warning
>  if (foo = bar) {}
>          ^
>      (        )
> t.c:2:11: note: use '==' to turn this assignment into an equality
> comparison
>  if (foo = bar) {}
>          ^
>          ==
>
> On Wed, Jun 1, 2016 at 8:59 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
> 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/20160620/e027b953/attachment.html>