Re: [PATCH v3 2/2] gdb/c++: Detect ambiguous variables in imported namespaces

[Tom Tromey <tom@tromey.com>]

>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

Bruno> When running gdb.cp/nsusing.cc and stopping at line 17, we can ask GDB
Bruno> to print x and get a compiler-dependent answer. Using gcc 12.2.1, GDB
Bruno> will print M::x, and using clang 16.0.0 prints N::x. Not only is this
Bruno> behavior confusing to users, it is also not consistent with compiler
Bruno> behaviors, which would warn that using x is ambiguous at this point.

Thanks for the patch.

Bruno> +      error (_("%s"), error_str.c_str ());
Bruno> +    }
Bruno> +  else
Bruno> +    return found_symbols;
Bruno> +
Bruno> +  /* This is needed to silence a -Werror=return-type warning, because
Bruno> +     the above if case doesn't have a return statement.  */
Bruno> +  gdb_assert_not_reached ();
Bruno>  }

This is surprising, because error is marked as noreturn.
Anyway perhaps a better workaround would be to just remove the 'else'.

Bruno> +  /* Despite getting a map, it should have at most one element, otherwise
Bruno> +     cp_lookup_symbol_via_import will have already reported the ambiguity.  */
Bruno> +  std::map<std::string, struct block_symbol> result
Bruno> +    = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1);

I think it would be better to have some kind of wrapper, where
cp_lookup_symbol_via_imports takes a set as a parameter (to avoid having
to merge sets), and then the wrapper extracts the result.

Maybe the error handling could be stuck in the wrapper as well.  I
didn't look into that.

Tom