Skip to content

Fix to #62194 broke the coherence tests and made error messages more mysterious #63145

Closed
@arielb1

Description

@arielb1

PR #62696 had removed the notes from the tests for the upstream/downstream printing logic, so now that logic is not tested:

2ced474

Plus, the new error messages are worse: an upstream impl can indeed be added, even if the crate is private.

I think that even in the OP case, some sort of note should be added, unless we change coherence. Again, the test case:

trait Private {
}

impl<T: Private> Send for T {
//~^ ERROR conflicting implementations
}

There, the error message is a coherence conflict for &_: Send, where this impl conflicts with the libcore impl, because the compiler can't prove that ! &_: Private.

Now, the error message says that the reason that the compiler can't prove ! &_ : Private is because of a possible downstream impl. Of course, no downstream impl can exist as the trait is private, however, I don't think we want to change coherence to account for that, especially not without an RFC.

Maybe a wording change is in order?

I don't think the "upstream" message needs to change any (see the test diff), it's still good in the private case. Maybe it should try to mention a non-private trait, but that would be some work.

Say, for non-exported trait, change the note to say:

note: unable to assume the type `&_` does not implement the trait `Private`

I'm not even sure the note is a problem, I personally like the state before the PR the best.

cc @estebank @nagisa

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-diagnosticsArea: Messages for errors, warnings, and lintsA-trait-systemArea: Trait systemC-bugCategory: This is a bug.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions