On Tue, Jan 3, 2017 at 8:28 PM, David Malcolm <dmalc...@redhat.com> wrote:
> PR c++/77829 and PR c++/78656 identify an issue within the C++ frontend
> where it issues nonsensical fix-it hints for misspelled name lookups
> within an explicitly given namespace: it finds the closest name within
> all namespaces, and uses the location of the namespace for the replacement,
> rather than the name.
>
> For example, for this error:
>
> #include <memory>
> void* allocate(std::size_t n)
> {
> return std::alocator<char>().allocate(n);
> }
>
> we currently emit an erroneous fix-it hint that would generate this
> nonsensical patch:
>
> {
> - return std::alocator<char>().allocate(n);
> + return allocate::alocator<char>().allocate(n);
> }
>
> whereas we ought to emit a fix-it hint that would generate this patch:
>
> {
> - return std::alocator<char>().allocate(n);
> + return std::allocator<char>().allocate(n);
> }
>
> This patch fixes the suggestions, in two parts:
>
> The incorrect name in the suggestion is fixed by introducing a
> new function "suggest_alternative_in_explicit_scope"
> for use by qualified_name_lookup_error when handling failures
> in explicitly-given namespaces, looking for hint candidates within
> just that namespace. The function suggest_alternatives_for gains a
> "suggest_misspellings" bool param, so that we can disable fuzzy name
> lookup for the case where we've ruled out hint candidates in the
> explicitly-given namespace.
>
> This lets us suggest "allocator" (found in "std") rather "allocate"
> (found in the global ns).
This looks good.
> The patch fixes the location for the replacement by updating
> local "unqualified_id" in cp_parser_id_expression from tree to
> cp_expr to avoid implicitly dropping location information, and
> passing this location to a new param of finish_id_expression.
> There are multiple users of finish_id_expression, and it wasn't
> possible to provide location information for the id for all of them
> so the new location information is assumed to be optional there.
> This fixes the underlined location, and ensures that the fix-it hint
> replaces "alocator", rather than "std".
I'm dubious about this approach, as I think this will fix some cases
and not others. The general problem is that we aren't sure what to do
with the location of a qualified-id: sometimes we use the location of
the unqualified-id, sometimes we use the beginning of the first
nested-name-specifier. I think the right answer is probably to use a
rich location with the caret on the unqualified-id. Then the fix-it
hint can use the caret location for the fix-it. Does that make sense?
Jason
