<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 4, 2020 at 11:14 AM Haojian Wu <<a href="mailto:hokein@google.com">hokein@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Wed, Nov 4, 2020 at 12:23 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 3, 2020 at 1:17 AM Haojian Wu <<a href="mailto:hokein@google.com" target="_blank">hokein@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Mon, Nov 2, 2020 at 7:09 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Mon, Nov 2, 2020 at 2:52 AM Haojian Wu <<a href="mailto:hokein@google.com" target="_blank">hokein@google.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thanks for all the replies. They are all useful.<div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">A major goal of Clang's diagnostic experience is that if a fixit is suggested (such as a typo correction) then subsequent errors are exactly as-if the fixit had been applied. It sounds like your suggestion would go counter to that?</blockquote><div><br></div><div>Thanks. I didn't notice that this is a clang goal for recovery. At the same time, this would mean we can't tell the difference from ASTs for non-typo-correct & typo-correct code because they are exactly the same.</div></div></div></blockquote><div><br></div><div>Yep</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Speaking of clangd, I should have given more information on why we want to suppress secondary diagnostics:<br></div><div>  - the provided case is from a bug report, and we have user complaints about it, see <a href="https://github.com/clangd/clangd/issues/547" target="_blank">https://github.com/clangd/clangd/issues/547</a></div><div>  - the idea of the "first" and "secondary" diagnostics is UI-specific, most LSP clients just group all diagnostics together, which makes the "first" diagnostic less obvious to users. The tricky case here is that we have multiple diagnostics with fix-its at the *same* position -- different LSP clients have different behavior, and they are suboptimal :(</div><div>     - vscode -- the quick-fix prompt-up widget just shows fix-it text, and there is no way to see which diagnostic does the fix-it fix</div><div>     - neovim builtin lsp -- this is worse, the typo-correct "did you mean free" diag is emitted on the "void free()" line rathan than "if (!force)" line</div></div></blockquote><div><br></div><div>Any chance of improving the consumers here?</div></div></div></blockquote><div><br></div><div>I hope so -- usually, we report bugs to the consumers. As a language server, we don't own them and from our experience, different LSP clients have different opinions, and we can't really fix the world :(</div></div></div></blockquote><div><br>Seems like if this interpretation of this part of the protocol causes confusion between servers and clients, it might be worth clarifying usage, etc, to help get some uniformity here.</div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> Alternatively/also, perhaps a general workaround/solution to clangd that strips fixits if they're at the same position?</div></div></div></blockquote><div></div></div><div><br></div><div>This is a possible solution, but it seems to workaround the issue rather than fixing it.</div></div></div></blockquote><div><br>I was thinking of it more as a more general fix - if part of the issue is multiple fixits on a single line, addressing that general problem (which could come up in non-spelling correction contexts too) would be useful?<br></div></div></div></blockquote><div><br></div><div>maybe, I think this is likely a UX issue.</div></div></div></blockquote><div><br>Sure enough - but it sounded like you were trying to fix that UX issue from clangd, and the issue arises whenever there's multiple fixits on a single line? So seems like addressing that generally might be good (either workaround in clangd, or pushing fixes upstream to clangd consumers (or workaround in clangd while getting fixes upstream before removing the workaround))<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>  - applying fix-its in interactive environments requires user confirmations, offering somewhat-speculative diagnostics nearby is a relatively worse tradeoff</div></div></blockquote><div><br></div><div>Not quite sure I follow this one - applying fix-its in a command line compiler situation requires user interaction too. </div></div></div></blockquote><div><br></div><div>oh, I didn't know this, can you say more about this?  My understanding of applying fix-its via command-line tools (e.g. clang -cc1 -fixit /path/fix.cc) is that it just applies all fix-its simultaneously, and there is no user interaction.<br></div></div></div></blockquote><div><br>In the sense that the compiler invoked in its default configuration doesn't apply any fixits -  you have to pass a flag to get it to apply fixits automatically (-fixit or something like that). It's true that once you pass this flag, it applies all fixits.<br></div></div></div></blockquote><div><br></div><div>OK, I see. I thought by "user interaction" you mean that the user can choose some fix-its to apply, but not just apply all.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>You mean some editors automatically prompt for application of fixits as soon as they're generated? Rather than the user, say, right-clicking and asking to apply them?<br></div></div></div></blockquote><div><br></div><div>I mean the latter one. In general, editors will highlight the range of diagnostics in some way, and the user can put a cursor in that range, and ask to apply fix-it.</div></div></div></blockquote><div><br>OK, then I'm a bit confused about the particular nature of the problem. Is it that the UI doesn't make it clear which fixits should be applied in what order? There are many non-typo-correction cases where clang would emit ordered fixits (where it doesn't make sense to apply one without applying the previous one) so I'm a bit confused about which issue is being solved.<br></div></div></div></blockquote><div><br></div><div>The problem I was talking about is that the editor UI for applying fix-its is suboptimal if there are > 1 diagnostics (error or warning) with fix-its on the same range.</div><div>Given the force/free example, we have "did you mean 'free'" error (with fixit1) and "always evaluate to 'true'" warning (with fixit2) on the same range, e.g. in vscode, a lightbulb icon will be displayed if the cursor is moved to the diagnostic range, by clicking the icon, it displays the two fix-its like below:</div><div>-  change 'force' to 'free' (fixit1)</div><div>-  prefix with the address-of operator to silence this warning (fixit2)</div><div>and use can select one to apply, the issue is that it is less clear to users that fixit1 is to fix the error, and fixit2 is to fix the warning. I think this is more likely a UX problem.</div></div></div></blockquote><div><br>*nod* But sounds like so long as that UX problem exists, you may not want to issue two fixits on the same line?<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> I think the issue we are discussing now is the bogus secondary diagnostic caused by our not-so-great typo-correction (force vs free).</div></div></div></blockquote><div><br>Right right.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Yeah, that would create a very different bar for fixits, I think. I'd potentially argue that that might not be a great editor default (like clang's -fixit isn't the default, but can be opted into).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>  - recompiling code in clangd is usually fast -- we have preamble optimization, and fix-its usually don't touch preamble region</div><div><br></div><div>Agree that we should improve the typo-correction heuristics, "force/free" is a bad typo-correction. I will try to see what I can do to make it better with some reasonable amount of work.</div><div><br></div><div>Regarding confidence levels of typo-correction, how about having two thresholds T1, T2:</div><div> - 0 < match < T1: suggest and recover (the current one is EditDistance/(length + 2) <= 1/3)</div><div> - T1 < match < T2: suggest but don't recover</div><div> - match > T2: don't suggest </div><div><br></div><div>And we can pick different values for clang vs clangd, we can turn the recover off by setting T1 = T2.<br></div></div></blockquote><div><br></div><div>I'd still be pretty hesitant to encourage/support the direction of turning off typo correction recovery (though I guess you mean "by setting T1 = 0", right?) but not fundamentally problematic to demote all the typo correction to note fixits - why not all fixits, then? Is there something specifically about spelling fixits that make them different from other fixits? Perhaps today, given the not-so-great edit distance calculation, the false positive rate for typo corrections may be too high and so even at low values of T1 there are still too many suspect typo corrections compared to other more manually implemented non-typo fixits.</div></div></div></blockquote><div><br></div><div>If I read your words correctly, we might have a misunderstanding about the "recover" term here. </div><div>I think what I meant is</div><div>1) 0 < match < T1 (high-confident mode): a typo fix-it on a diagnostic error, and full recovery in the AST (the AST is exactly the same as the typo fix-it is applied).</div><div>2) T1 < match < T2 (less-confident mode): a typo fix-it on a diagnostic note, and no full recovery in the AST (with RecoveryExpr).</div><div>3) match > T2: no typo-correction at all.</div><div><br></div><div>Currently we have 1) and 3) in clang. 2) is the missing part, and I think it matches what you suggested before.</div></div></div></blockquote><div><br>Yes, I think it does - I think there were a couple of issues. "we can turn the recover off by setting T1 = T2" seems to be incorrect. Setting T1 = T2 would /remove/ (2), the entire thing we're discussing creating. So I assume you meant "we can turnt he recover off by setting T1 = 0" so that (1) becomes empty (nothing matches in that category) and everything becomes (2). </div></div></div></blockquote><div><br></div><div>Yeah, you're right. sorry, my math is worse than I thought.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>I don't think it'd be a great user experience to turn off (1) entirely and as much as possible I think it'd be good not to diverge too much between the command line compiler and the IDE integration - so I'd be in favor of doing more to improve the typo correction to help make (1) high quality and (2) lower quality fairly consistently, and then support that both on the command line and via clangd with the same values for T1 and T2.<br><br>But if improving the typo correction quality is too much of a rabbit hole, yes, maybe adding these configurations would be good - but even then, perhaps we should make clang use the values that clangd uses here, and offer less recovery typo corrections until they can be made more reliable.<br></div></div></div></blockquote><div><br></div><div>Fully agree. I think our plan would be </div><div>1. improve the typo-correction quality</div><div>2. add the two-threshold mechanism in clang</div><div><br></div><div>These two can be done in parallel. I will see what kind of improvement I can do with some reasonable amount of work.</div></div></div></blockquote><div><br></div><div>Sounds good!<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 30, 2020 at 9:36 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 30, 2020 at 1:31 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Fri, 30 Oct 2020 at 11:37, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Fri, Oct 30, 2020 at 11:21 AM Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Fri, 30 Oct 2020 at 10:15, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">A major goal of Clang's diagnostic experience is that if a fixit is suggested (such as a typo correction) then subsequent errors are exactly as-if the fixit had been applied. It sounds like your suggestion would go counter to that?<br><br>I think there's probably a good argument to be made that not all typo corrections are high-confidence enough to merit a fixit on the error itself - if the fixit is on a note instead, the above requirement of recovery isn't applicable (so that's where we can put, say "did you mean if (a == b)" as well as "did you mean if ((a = b))" fixits on alternative notes on the general -Wparentheses warning) - so perhaps having some level of typo correction confidence would be useful to determine which kind of recovery we should do - full recovery as if the user wrote the code (with a fixit hint attached to the error itself) or "well, we're not sure but here's out best guess" where an invalid expr is created and the fixit hint is attached to a note with some wording that's a bit more vague/conveys the increased uncertainty compared to the former case.</div></div></blockquote><div><br></div><div>Right. It's an explicit goal to recover as if the typo-correction is applied, in the case where we're confident that it's right. Currently we get that confidence by checking the enclosing context in which the typo appears is valid once the correction is applied. But that's imperfect in various ways -- one of them is that the context we check is a little too narrow sometimes; another (the issue in this case) is that making the enclosing context be valid is not really sufficient to know that the typo correction actually makes sense.</div><div><br></div><div>Perhaps we could add some further heuristics to determine whether the result of typo correction seems reasonable before deciding we're confident it's correct; I could imagine, for example, annotating warnings with a "causes typo correction to be considered 'bad'" flag, in much the same way as we have a "causes SFINAE failure" flag, and using that to validate corrections -- that is, reject typo corrections not only if they would make the code invalid, but also if they would produce a warning that suggests the code is unlikely to be what the user intended. (In this case I think the warning is actually produced after we've finished correcting the typo, though that's probably not all that hard to fix.)</div></div></div></blockquote><div><br></div><div>Sounds plausible to me - what do you think about the typo correction itself being a bit more reserved about what constitutes a recoverable typo correction? If the edit distance is too far maybe we shouldn't be suggesting it, or should be suggesting it at lower priority (like force V free below - I mean, I appreciate the suggestion if that's the nearest thing, but even if it did make the code compile without any further warnings, I'm not sure it's a sufficiently good guess to recover with it?)</div></div></div></blockquote><div><br></div><div>I don't think we have the edit distance computation right yet. force -> free looks to the edit distance algorithm like two errors (each with a cost of 1): a spurious 'o' and a 'c' where an 'e' was meant (falling below the threshold of (length+2)/3 == 2), but to a human they look like completely different words, whereas getDist -> getDistance looks like 4 errors, which exceeds the threshold of (7+2)/3 == 3, but to a human would look like very-likely-correct suggestion.</div><div><br></div><div>We'd probably do a lot better if we treated a run of length N of consecutive additions / removals as having a lower cost than N independent additions / removals. Similarly, adjacent transposed characters should have a lower cost than a removal plus an addition / two replacements (which is how it's currently weighted) -- and should probably have a lower cost than a single addition or removal. And perhaps a doubled letter should have a lower cost than a general spurious letter (but maybe I only think that because the keyboard on my laptop is misbehaving!).</div><div><br></div><div>There are other obvious-to-a-human mistakes we make as part of the edit distance algorithm (for example, suggesting that `get` might be a typo for `set` or vice versa). There might be broadly-applicable rules we could use to detect those; for example, we could divide identifiers into "words" by splitting on underscores and lowercase -> uppercase transitions, and never treat a word as containing a typo if we've seen that word elsewhere in the compilation -- so we wouldn't suggest that "getFoo" is a typo for "setFoo" if we've seen the word "get" appearing in some identifier already, but we would treat "aetFoo" as a typo for "setFoo". We could in principle get really smart and notice that (for example) "get" and "set" mean different things (because we've seen "getFoo" and "setFoo" overloaded) but that "get" and "is" aren't known to mean different things (because we've never seen "getFoo" and "isFoo" overloaded), and so decide that "getFoo" is a typo for "isFoo" not "setFoo". But that's probably excessively complicated.</div><div><br></div><div>There are also heuristics we could apply based on keyboard layouts -- spurious letters are more likely if they're on keycaps that are adjacent to the next or previous letter, and replacements are similarly more likely if they're on adjacent keycaps -- but in general we don't know what keyboard layout the user is using.</div></div></div></blockquote><div><br></div><div>Ah, fair points all - yeah, maybe the higher prioritiy/bigger bang-for-buck today will be improvements to the edit distance algorithm itself, before we get to the point of benefiting from the sort of nuanced separation/classification of relative certainty.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>But even with the best edit distance algorithm, I think you're right that we can't set a hard cutoff and say "anything better than this that's valid when substituted into the local context is definitely right; anything worse than this is definitely wrong", and having a middle ground of "we're not sure this correction is good enough so we're not going to use it for error recovery / fix-its" might make sense.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 30, 2020 at 7:34 AM Haojian Wu via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hello folks, </div><div><br></div><div>Given the following case:</div><div><br></div><div>void free();<br></div><div>void test() {</div><div>   if (!force) {} // diagnostic 1:  use of undeclared identifier 'force'; did you mean 'free'?</div><div>                     // diagnostic 2:  warning: address of function 'free' will always evaluate to 'true'</div><div>}</div><div><br></div><div>The secondary diagnostic seems to be bogus, and it doesn't reflect the written source code, which can easily cause confusions. My idea is to use a dependent RecoveryExpr (which wraps the typo-correct AST node) to suppress all secondary diagnostics.</div><div><br></div><div>I have a prototype at <a href="https://reviews.llvm.org/D90459" target="_blank">https://reviews.llvm.org/D90459</a>. I see some improvements, but there are some regressions as well:</div><div><br></div><div>Improvements</div><div>- the resulting AST look better because the error is visible in the AST (with RecoveryExpr node)</div><div>- we emit more typo corrections for more cases, see <a href="https://reviews.llvm.org/differential/changeset/?ref=2240247" target="_blank">[1]</a>, <a href="https://reviews.llvm.org/differential/changeset/?ref=2240248" target="_blank">[2]</a></div><div><br></div><div>Regressions</div><div>- recursive/nested typo corrections, e.g. `TypoX.TypoY;`, we emit just 1 typo-correction while the old behavior emits two, see <a href="https://reviews.llvm.org/differential/changeset/?ref=2240254" target="_blank">[1]</a></div><div></div><div>- ambiguous typos, when there are ambiguous typo candidates (they have the same edit distance), the old one seems to perform better in some cases, see<a href="https://reviews.llvm.org/differential/changeset/?ref=2240246" target="_blank"> [1]</a></div><div>- other misc regressions, I think we could fix them</div><div><br></div><div>The secondary diagnostics are not wrong from the AST perspective, but they seem to be unnecessary. In clangd, we'd like to suppress all secondary diagnostics, but I'm not sure this is a right choice for clang.</div></div></blockquote><div><br></div><div>That would seem unfortunate to me - clang works pretty hard on diagnostic recovery so users can see/act on multiple diagnostics in one pass. Though I realize that model is a bit different if you're dealing with an editor that's recompiling after every textual change - is that always the case for clangd? I think it might still be useful to see more than one error in an IDE/editor's error list, and certainly if I were dealing with some code that's slow to compile or an editor that chooses to do less fine-grained recompiles.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>What do people think?</div><div><br></div><div><br></div><div>Thanks,</div><div>Haojian</div><div><br></div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>