<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><br><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">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>