On 15 September 2015 at 17:38, David Malcolm <dmalc...@redhat.com> wrote: > It would be appropriate to exit as soon as we reach 1 if we agree > that lookup_name_fuzzy isn't intended to find exact matches (since > otherwise we might fail to return an exact match if we see a > distance 1 match first). > > I haven't implemented that early bailout in this iteration of the > patch; should I?
Everything I say are mere suggestions since I cannot approve patches, so I would say: "whatever the approvers say!" :-) Nevertheless, how an exact match would play out? unknown type name 'inp'; did you mean 'inp'? > Remaining work: > * the FIXME about where to call levenshtein_distance_unit_tests; > there's an argument that this could be moved to libiberty (is C++ > allowed in libiberty?); I'd prefer to get the unittest idea from > https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00765.html > into trunk, and then move it into there. Right now it's all > gcc_assert, so optimizes away in a production build. That would be gcc_checking_assert, no? gcc_assert() should still work in release mode, AFAIU. > * try existing testcases as noted by Manu above I think the most useful part of checking those is that we have really wacky testcases and it may show cases where things go horribly wrong. Plus, if the suggestion is perfect, then you have another testcase for free. This is what I was doing with the Wformat precise locations. > It also strikes me that sometimes a "misspelling" is a missing > header file, and that the most helpful thing to do might be to > suggest including that header file. For instance given: > $ cat /tmp/foo.c > int64_t i; > > $ ./xgcc -B. /tmp/foo.c > /tmp/foo.c:1:1: error: unknown type name ‘int64_t’ > int64_t i; > ^ > (where the suggestion of "int" is suppressed due to the distance > being too long) it might be helpful to print: > /tmp/foo.c:1:1: error: unknown type name 'int64_t'; did you mean to include > '<inttypes.h>'? > int64_t i; > ^ > That does seem like a separate enhancement, though. We already suggest header files for built-in functions: https://gcc.gnu.org/PR59717 Doing the same for "standard" types would not be a stretch, but yes, it is a separate thing. > diff --git a/gcc/spellcheck.c b/gcc/spellcheck.c > new file mode 100644 > index 0000000..c407aa0 > --- /dev/null > +++ b/gcc/spellcheck.c > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "tm.h" > +#include "tree.h" > +#include "spellcheck.h" Why tm.h? Great work! Manuel.
