Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCIP symbols for inherent impl declarations are ambiguous #18772

Open
mgsloan opened this issue Dec 28, 2024 · 2 comments
Open

SCIP symbols for inherent impl declarations are ambiguous #18772

mgsloan opened this issue Dec 28, 2024 · 2 comments

Comments

@mgsloan
Copy link
Contributor

mgsloan commented Dec 28, 2024

In #18758 the plan is to only include the first SymbolInformation in this case. An alternative might be to number them (and append these numbers using Meta SCIP descriptors), but this is tricky with how static_index.rs is implemented.

Not sure how important this is, since inherent impl declarations are not really referenceable. They do show up in symbol names, though, and some consumers of SCIP might expect to be able to resolve prefixes of SCIP symbols.

@Veykril
Copy link
Member

Veykril commented Dec 29, 2024

Numbering would be ideal in my eyes. How are trait impls handled, that is if you had a impl Trait<Bar> for Foo and impl Trait<Qux> for Foo?

@mgsloan
Copy link
Contributor Author

mgsloan commented Dec 29, 2024

@Veykril The current code is buggy there and those traits would alias each-other, both are Foo#Trait#. With my improvements in #18758, they're instead impl#[Foo][`Trait<Bar>`] and impl#[Foo][`Trait<Qux>`]. I believe this is typically unique, but I suppose it is not in the presence of specialization. However, specialization is unstable and doesn't seem to have a clear path towards stabilization afaict.

I checked, and trait impls cannot be split into multiple declarations the way that inherent impls can, so the numbering does not apply.

To implement numbering I believe this would need to get added to the nodes at parse time. I do not want to figure out how to implement that, and I would strongly prefer to not block merging #18758 on such concerns. Even having a way to refer to inherent impl blocks is kinda a niche thing - what's the downstream usecase? I guess maybe as a place to hang docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants