-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Description
Display
and DisplayWithEngines
for CallPath
simply concatenate the call path prefixes and the suffix, without being aware of any contextual information, like, if the path is a full path, or an ambiguos one, or for which purpose is the string representation of the path requested.
This is a caveat because it leads to generation of paths which are invalid in a particular context. E.g., the string representation of the full path [package, mod_a, mod_b, Item]
within the package
is ::mod_a::mod_b::Item
and not package::mod_a::mod_b::Item
.
Note that Display
and DisplayWithEngines
do not have access to the namespace
within which the path is observed and thus cannot display path with the complete contextual information.
This is error-prone because methods like to_string_with_arg
in #6871 can unintentionally use Display
to get the path which will in many cases look like the expected path, but will be, e.g., lacking leading ::
when needed.
Also, we have cases like this one where the callers are burdend with writing cumbersome code to decide if they should put ::
as a prefix or not.
We need to review the usages of Display
and DisplayWithEngines
for CallPath
to check what kind of a path is actually expected in those usages, in respect to the CallPathType
and the current namespace and module.
The proposal is to add dedicated to_xyz_string
methods to CallPath
that will accept namespace
parameter and return the string properly formated to the context and usage. E.g., to_full_use_path_string
.
The proposal is also, to fully remove the Display
and DisplayWithEngines
implementations and leave only Debug
and DebugWithEngines
. That way we can avoid unintentional error-prone usage and force the usage of the intended to_xyz_string
method. Removal might be an issue though, as Display
and DisplayWithEngines
might be called recurively inside of other Display
implementations. Here, I see a general question of the semantic correctness of such "parent" Display/WithEngine
implementations.