Skip to content

prevent negative impl cycles #102678

Open
Open
@nikomatsakis

Description

@nikomatsakis

When integrating negative impls into a-mir-formality, I realized a problem with the current check.

The current logic has a check that, at its heart, boils down to this:

  • If there are two overlapping impls and one impl requires T: Trait, and we can prove T: !Trait, then this impl can be ignored.

This assumes that the coherence checker is guaranteeing that T: Trait and T: !Trait do not hold for the same T. This is generally true, but there is a trick, because this code is running during coherence. So you can wind up with the coherence check for Trait (which has the job of proving that T: Trait and T: !Trait do not overlap) also relying on that same condition.

In the current trait checker, this leads to a cycle, but in a-mir-formality, which permits cycles, it leads to unsoundness. Given that I plan to propose that we allow cycles for all traits (i.e., coinductive semantics), this is a problem.

Here is an example test:

trait MyTrait { }

struct Foo { }

impl !MyTrait for Foo { }

impl MyTrait for Foo
where
    Foo: MyTrait // in formality, this would work
{ }

What happens in this test:

  • We see that the two impls overlap.
  • We see that the second impl has Foo: MyTrait and try to prove Foo: !MyTrait, which succeeds (there is a negative impl, after all).
  • We conclude that the two impls are disjoint (but that's not true, obviously).

I'll leave discussion on the best way to fix this for issue text.

Metadata

Metadata

Assignees

No one assigned

    Labels

    F-negative_impls#![feature(negative_impls)]I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-lowLow priorityT-typesRelevant to the types team, which will review and decide on the PR/issue.requires-nightlyThis issue requires a nightly compiler in some way.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions