Skip to content

Review usages of Display/WithEngines for CallPath and provide dedicated to_xyz_string methods instead #6873

@ironcev

Description

@ironcev

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    compilerGeneral compiler. Should eventually become more specific as the issue is triagedcompiler: frontendEverything to do with type checking, control flow analysis, and everything between parsing and IRgenteam:compilerCompiler Team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions