[cfe-dev] Suppress secondary diagnostics for typo correction

[Haojian Wu via cfe-dev]

Hello folks,

Given the following case:

void free();
void test() {
   if (!force) {} // diagnostic 1:  use of undeclared identifier 'force';
did you mean 'free'?
                     // diagnostic 2:  warning: address of function 'free'
will always evaluate to 'true'
}

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.

I have a prototype at https://reviews.llvm.org/D90459. I see some
improvements, but there are some regressions as well:

Improvements
- the resulting AST look better because the error is visible in the AST
(with RecoveryExpr node)
- we emit more typo corrections for more cases, see [1]
<https://reviews.llvm.org/differential/changeset/?ref=2240247>, [2]
<https://reviews.llvm.org/differential/changeset/?ref=2240248>

Regressions
- recursive/nested typo corrections, e.g. `TypoX.TypoY;`, we emit just 1
typo-correction while the old behavior emits two, see [1]
<https://reviews.llvm.org/differential/changeset/?ref=2240254>
- 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
[1] <https://reviews.llvm.org/differential/changeset/?ref=2240246>
- other misc regressions, I think we could fix them

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.

What do people think?


Thanks,
Haojian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201030/306b1986/attachment.html>