On 10/09/15 22:28, David Malcolm wrote:
There are a couple of FIXMEs here:
* where to call levenshtein_distance_unit_tests
Should this be part of make check? Perhaps a small program that is compiled and
linked with spellcheck.c? This would be possible if spellcheck.c did not depend
on tree.h or tm.h, which I doubt it needs to.
* should we attempt error-recovery in c-typeck.c:build_component_ref
I would say yes, but why not leave this discussion to a later patch? The
current one seems useful enough.
+
+/* Look for the closest match for NAME within the currently valid
+ scopes.
+
+ This finds the identifier with the lowest Levenshtein distance to
+ NAME. If there are multiple candidates with equal minimal distance,
+ the first one found is returned. Scopes are searched from innermost
+ outwards, and within a scope in reverse order of declaration, thus
+ benefiting candidates "near" to the current scope. */
+
+tree
+lookup_name_fuzzy (tree name)
+{
+ gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+
+ c_binding *best_binding = NULL;
+ int best_distance = INT_MAX;
+
+ for (c_scope *scope = current_scope; scope; scope = scope->outer)
+ for (c_binding *binding = scope->bindings; binding; binding =
binding->prev)
+ {
+ if (!binding->id)
+ continue;
+ int dist = levenshtein_distance (name, binding->id);
+ if (dist < best_distance)
I guess 'dist' cannot be negative. Can it be zero? If not, wouldn't be
appropriate to exit as soon as it becomes 1?
Is this code discriminating between types and names? That is, what happens for:
typedef int ins;
int foo(void)
{
int inr;
inp x;
}
+/* Recursively append candidate IDENTIFIER_NODEs to CANDIDATES. */
+
+static void
+lookup_field_fuzzy_find_candidates (tree type, tree component,
+ vec<tree> *candidates)
+{
+ tree field;
+ for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
+ {
+ if (DECL_NAME (field) == NULL_TREE
+ && (TREE_CODE (TREE_TYPE (field)) == RECORD_TYPE
+ || TREE_CODE (TREE_TYPE (field)) == UNION_TYPE))
+ {
+ lookup_field_fuzzy_find_candidates (TREE_TYPE (field),
+ component,
+ candidates);
+ }
+
+ if (DECL_NAME (field))
+ candidates->safe_push (field);
+ }
+}
This is appending inner-most, isn't it? Thus, given:
struct s{
struct j { int aa; } kk;
int aa;
};
void foo(struct s x)
{
x.ab;
}
it will find s::j::aa before s::aa, no?
tree
-build_component_ref (location_t loc, tree datum, tree component)
+build_component_ref (location_t loc, tree datum, tree component,
+ source_range *ident_range)
{
tree type = TREE_TYPE (datum);
enum tree_code code = TREE_CODE (type);
@@ -2294,7 +2356,31 @@ build_component_ref (location_t loc, tree datum, tree
component)
if (!field)
{
- error_at (loc, "%qT has no member named %qE", type, component);
+ if (!ident_range)
+ {
+ error_at (loc, "%qT has no member named %qE",
+ type, component);
+ return error_mark_node;
+ }
+ gcc_rich_location richloc (*ident_range);
+ if (TREE_CODE (datum) == INDIRECT_REF)
+ richloc.add_expr (TREE_OPERAND (datum, 0));
+ else
+ richloc.add_expr (datum);
+ field = lookup_field_fuzzy (type, component);
+ if (field)
+ {
+ error_at_rich_loc
+ (&richloc,
+ "%qT has no member named %qE; did you mean %qE?",
+ type, component, field);
+ /* FIXME: error recovery: should we try to keep going,
+ with "field"? (having issued an error, and hence no
+ output). */
+ }
+ else
+ error_at_rich_loc (&richloc, "%qT has no member named %qE",
+ type, component);
return error_mark_node;
}
I don't understand why looking for a candidate or not depends on ident_range.
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck.c
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+struct foo
+{
+ int foo;
+ int bar;
+ int baz;
+};
+
+int test (struct foo *ptr)
+{
+ return ptr->m_bar; /* { dg-error "'struct foo' has no member named 'm_bar'; did
you mean 'bar'?" } */
+
+/* { dg-begin-multiline-output "" }
+ return ptr->m_bar;
+ ~~~ ^~~~~
+ { dg-end-multiline-output "" } */
+}
+
+int test2 (void)
+{
+ struct foo instance = {};
+ return instance.m_bar; /* { dg-error "'struct foo' has no member named 'm_bar';
did you mean 'bar'?" } */
+
+/* { dg-begin-multiline-output "" }
+ return instance.m_bar;
+ ~~~~~~~~ ^~~~~
+ { dg-end-multiline-output "" } */
+}
+
+int64 foo; /* { dg-error "unknown type name 'int64'; did you mean 'int'?" } */
+/* { dg-begin-multiline-output "" }
+ int64 foo;
+ ^~~~~
+ { dg-end-multiline-output "" } */
These tests could also test different scopes, clashes between types and fields
and variables, and the correct behavior for nested struct/unions.
I wonder whether it would be worth it to extend existing tests if now they emit
the "do you mean" part to be sure they are doing the right thing.
Cheers,
Manuel.
