From c3797dcce68a0b8b51028f2b9c797d300bac94c1 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 16 Mar 2022 14:59:03 -0300 Subject: [PATCH 01/12] Add double negative trait test case --- .../coherence/coherence-overlap-double-negative.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/test/ui/coherence/coherence-overlap-double-negative.rs diff --git a/src/test/ui/coherence/coherence-overlap-double-negative.rs b/src/test/ui/coherence/coherence-overlap-double-negative.rs new file mode 100644 index 0000000000000..1ea0ddc7430e1 --- /dev/null +++ b/src/test/ui/coherence/coherence-overlap-double-negative.rs @@ -0,0 +1,12 @@ +// check-pass + +#![feature(negative_impls)] +#![feature(with_negative_coherence)] + +trait A {} +trait B: A {} + +impl !A for u32 {} +impl !B for u32 {} + +fn main() {} From 972ee874324db2b373bec6c807a763f44a6eac6a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 16 Mar 2022 16:18:47 +0100 Subject: [PATCH 02/12] Update browser-ui-test version to 0.8.3 --- src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile index b0f052e6cf0d9..d78fc6d208330 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile @@ -72,7 +72,7 @@ ENV PATH="/node-v14.4.0-linux-x64/bin:${PATH}" # https://github.com/puppeteer/puppeteer/issues/375 # # We also specify the version in case we need to update it to go around cache limitations. -RUN npm install -g browser-ui-test@0.8.1 --unsafe-perm=true +RUN npm install -g browser-ui-test@0.8.3 --unsafe-perm=true ENV RUST_CONFIGURE_ARGS \ --build=x86_64-unknown-linux-gnu \ From 45a3075c5618935c86040c2a9debf72afab90e24 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 16 Mar 2022 22:22:16 +0100 Subject: [PATCH 03/12] Run GUI test when browser-ui-test version is updated --- src/ci/scripts/should-skip-this.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ci/scripts/should-skip-this.sh b/src/ci/scripts/should-skip-this.sh index bb48fcb5a216e..c046a6c8a26f3 100755 --- a/src/ci/scripts/should-skip-this.sh +++ b/src/ci/scripts/should-skip-this.sh @@ -25,6 +25,7 @@ if [[ -n "${CI_ONLY_WHEN_SUBMODULES_CHANGED-}" ]]; then elif ! (git diff --quiet "$BASE_COMMIT" -- \ src/test/rustdoc-gui \ src/librustdoc \ + src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile \ src/tools/rustdoc-gui); then # There was a change in either rustdoc or in its GUI tests. echo "Rustdoc was updated" From 78346489c66779d024c60af2d2f0f5d9d455f688 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 11:10:19 -0300 Subject: [PATCH 04/12] Add comments on Polarity --- compiler/rustc_middle/src/ty/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index e7cef9e198b6f..83ab761aa55a4 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -748,6 +748,13 @@ pub struct TraitPredicate<'tcx> { pub constness: BoundConstness, + /// If polarity is Positive: we are proving that the trait is implemented. + /// + /// If polarity is Negative: we are proving that a negative impl of this trait + /// exists. (Note that coherence also checks whether negative impls of supertraits + /// exist via a series of predicates.) + /// + /// If polarity is Reserved: that's a bug. pub polarity: ImplPolarity, } From 64dfd3b2340a42f5fda5802d50c9739d38533102 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 11:26:45 -0300 Subject: [PATCH 05/12] Make negative coherence work when there's impl negative on super predicates --- .../src/traits/coherence.rs | 24 ++++++++++++------- .../coherence-overlap-super-negative.rs | 18 ++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/coherence/coherence-overlap-super-negative.rs diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index a3d32acc6fb82..72a06abd8e034 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -18,7 +18,7 @@ use rustc_errors::Diagnostic; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::CRATE_HIR_ID; use rustc_infer::infer::TyCtxtInferExt; -use rustc_infer::traits::TraitEngine; +use rustc_infer::traits::{util, TraitEngine}; use rustc_middle::traits::specialization_graph::OverlapMode; use rustc_middle::ty::fast_reject::{self, TreatParams}; use rustc_middle::ty::fold::TypeFoldable; @@ -353,6 +353,7 @@ fn negative_impl<'cx, 'tcx>( }) } +#[instrument(level = "debug", skip(selcx))] fn negative_impl_exists<'cx, 'tcx>( selcx: &SelectionContext<'cx, 'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -361,14 +362,18 @@ fn negative_impl_exists<'cx, 'tcx>( ) -> bool { let infcx = &selcx.infcx().fork(); let tcx = infcx.tcx; - o.flip_polarity(tcx) - .map(|o| { + + let super_obligations = util::elaborate_predicates(tcx, iter::once(o.predicate)); + + for o in iter::once(o.clone()).chain(super_obligations) { + if let Some(o) = o.flip_polarity(tcx) { let mut fulfillment_cx = FulfillmentContext::new(); fulfillment_cx.register_predicate_obligation(infcx, o); let errors = fulfillment_cx.select_all_or_error(infcx); + if !errors.is_empty() { - return false; + continue; } let mut outlives_env = OutlivesEnvironment::new(param_env); @@ -389,13 +394,16 @@ fn negative_impl_exists<'cx, 'tcx>( let errors = infcx.resolve_regions(region_context, &outlives_env, RegionckMode::default()); + if !errors.is_empty() { - return false; + continue; } - true - }) - .unwrap_or(false) + return true; + } + } + + false } pub fn trait_ref_is_knowable<'tcx>( diff --git a/src/test/ui/coherence/coherence-overlap-super-negative.rs b/src/test/ui/coherence/coherence-overlap-super-negative.rs new file mode 100644 index 0000000000000..d296a094a3704 --- /dev/null +++ b/src/test/ui/coherence/coherence-overlap-super-negative.rs @@ -0,0 +1,18 @@ +// check-pass + +#![feature(negative_impls)] +#![feature(rustc_attrs)] +#![feature(with_negative_coherence)] + +trait Trait1: Trait2 {} +trait Trait2 {} + +struct MyType {} +impl !Trait2 for MyType {} + +#[rustc_strict_coherence] +trait Foo {} +impl Foo for T {} +impl Foo for MyType {} + +fn main() {} From 61a05ef8d6a0f5b506dc0d29ef04036238a474a9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 12:09:00 -0300 Subject: [PATCH 06/12] Extract obligation resolution to function --- .../src/traits/coherence.rs | 76 +++++++++++-------- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 72a06abd8e034..595337b2e91d8 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -17,7 +17,7 @@ use crate::traits::{ use rustc_errors::Diagnostic; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::CRATE_HIR_ID; -use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::{util, TraitEngine}; use rustc_middle::traits::specialization_graph::OverlapMode; use rustc_middle::ty::fast_reject::{self, TreatParams}; @@ -361,46 +361,62 @@ fn negative_impl_exists<'cx, 'tcx>( o: &PredicateObligation<'tcx>, ) -> bool { let infcx = &selcx.infcx().fork(); - let tcx = infcx.tcx; - let super_obligations = util::elaborate_predicates(tcx, iter::once(o.predicate)); + if resolve_negative_obligation(infcx, param_env, region_context, o) { + return true; + } - for o in iter::once(o.clone()).chain(super_obligations) { - if let Some(o) = o.flip_polarity(tcx) { - let mut fulfillment_cx = FulfillmentContext::new(); - fulfillment_cx.register_predicate_obligation(infcx, o); + for o in util::elaborate_predicates(infcx.tcx, iter::once(o.predicate)) { + if resolve_negative_obligation(infcx, param_env, region_context, &o) { + return true; + } + } - let errors = fulfillment_cx.select_all_or_error(infcx); + false +} - if !errors.is_empty() { - continue; - } +#[instrument(level = "debug", skip(infcx))] +fn resolve_negative_obligation<'cx, 'tcx>( + infcx: &InferCtxt<'cx, 'tcx>, + param_env: ty::ParamEnv<'tcx>, + region_context: DefId, + o: &PredicateObligation<'tcx>, +) -> bool { + let tcx = infcx.tcx; - let mut outlives_env = OutlivesEnvironment::new(param_env); - // FIXME -- add "assumed to be well formed" types into the `outlives_env` + if let Some(o) = o.flip_polarity(tcx) { + let mut fulfillment_cx = FulfillmentContext::new(); + fulfillment_cx.register_predicate_obligation(infcx, o); - // "Save" the accumulated implied bounds into the outlives environment - // (due to the FIXME above, there aren't any, but this step is still needed). - // The "body id" is given as `CRATE_HIR_ID`, which is the same body-id used - // by the "dummy" causes elsewhere (body-id is only relevant when checking - // function bodies with closures). - outlives_env.save_implied_bounds(CRATE_HIR_ID); + let errors = fulfillment_cx.select_all_or_error(infcx); - infcx.process_registered_region_obligations( - outlives_env.region_bound_pairs_map(), - Some(tcx.lifetimes.re_root_empty), - param_env, - ); + if !errors.is_empty() { + return false; + } - let errors = - infcx.resolve_regions(region_context, &outlives_env, RegionckMode::default()); + let mut outlives_env = OutlivesEnvironment::new(param_env); + // FIXME -- add "assumed to be well formed" types into the `outlives_env` - if !errors.is_empty() { - continue; - } + // "Save" the accumulated implied bounds into the outlives environment + // (due to the FIXME above, there aren't any, but this step is still needed). + // The "body id" is given as `CRATE_HIR_ID`, which is the same body-id used + // by the "dummy" causes elsewhere (body-id is only relevant when checking + // function bodies with closures). + outlives_env.save_implied_bounds(CRATE_HIR_ID); - return true; + infcx.process_registered_region_obligations( + outlives_env.region_bound_pairs_map(), + Some(tcx.lifetimes.re_root_empty), + param_env, + ); + + let errors = infcx.resolve_regions(region_context, &outlives_env, RegionckMode::default()); + + if !errors.is_empty() { + return false; } + + return true; } false From 91846fe12ad2296b6acc808e79f89020ddd194eb Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 12:36:12 -0300 Subject: [PATCH 07/12] This test now works --- .../rustc_trait_selection/src/traits/coherence.rs | 1 + .../coherence-overlap-negate-alias-strict.rs | 4 ++-- .../coherence-overlap-negate-alias-strict.stderr | 11 ----------- 3 files changed, 3 insertions(+), 13 deletions(-) delete mode 100644 src/test/ui/coherence/coherence-overlap-negate-alias-strict.stderr diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 595337b2e91d8..63aacbe2e8563 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -366,6 +366,7 @@ fn negative_impl_exists<'cx, 'tcx>( return true; } + // FIXME we should implement an `assemble_neg_candidates` fn for o in util::elaborate_predicates(infcx.tcx, iter::once(o.predicate)) { if resolve_negative_obligation(infcx, param_env, region_context, &o) { return true; diff --git a/src/test/ui/coherence/coherence-overlap-negate-alias-strict.rs b/src/test/ui/coherence/coherence-overlap-negate-alias-strict.rs index c240a18398278..48dffc921a31b 100644 --- a/src/test/ui/coherence/coherence-overlap-negate-alias-strict.rs +++ b/src/test/ui/coherence/coherence-overlap-negate-alias-strict.rs @@ -1,3 +1,5 @@ +// check-pass + #![feature(negative_impls)] #![feature(rustc_attrs)] #![feature(trait_alias)] @@ -13,7 +15,5 @@ impl !A for u32 {} trait C {} impl C for T {} impl C for u32 {} -//~^ ERROR: conflicting implementations of trait `C` for type `u32` [E0119] -// FIXME this should work, we should implement an `assemble_neg_candidates` fn fn main() {} diff --git a/src/test/ui/coherence/coherence-overlap-negate-alias-strict.stderr b/src/test/ui/coherence/coherence-overlap-negate-alias-strict.stderr deleted file mode 100644 index 30d837a5c5019..0000000000000 --- a/src/test/ui/coherence/coherence-overlap-negate-alias-strict.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0119]: conflicting implementations of trait `C` for type `u32` - --> $DIR/coherence-overlap-negate-alias-strict.rs:15:1 - | -LL | impl C for T {} - | ------------------- first implementation here -LL | impl C for u32 {} - | ^^^^^^^^^^^^^^ conflicting implementation for `u32` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0119`. From 9c076f3f90444183d4d51754f19ea55a801e0e05 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 12:37:18 -0300 Subject: [PATCH 08/12] Add more commments --- compiler/rustc_trait_selection/src/traits/coherence.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 63aacbe2e8563..60111446b946e 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -353,6 +353,7 @@ fn negative_impl<'cx, 'tcx>( }) } +/// Try to prove that a negative impl exist for the given obligation and their super predicates. #[instrument(level = "debug", skip(selcx))] fn negative_impl_exists<'cx, 'tcx>( selcx: &SelectionContext<'cx, 'tcx>, @@ -366,6 +367,7 @@ fn negative_impl_exists<'cx, 'tcx>( return true; } + // Try to prove a negative obligation exist for super predicates // FIXME we should implement an `assemble_neg_candidates` fn for o in util::elaborate_predicates(infcx.tcx, iter::once(o.predicate)) { if resolve_negative_obligation(infcx, param_env, region_context, &o) { From 30fbcdb3c40f4168e02b9dfab71ae2976011096c Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Fri, 18 Mar 2022 02:54:06 +0900 Subject: [PATCH 09/12] refactor: remove an unnecessary pattern for ignoring all parts --- compiler/rustc_resolve/src/late.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 91695257137cb..d5b1aa00e52de 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1129,7 +1129,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } for param in &generics.params { - if let GenericParamKind::Lifetime { .. } = param.kind { + if let GenericParamKind::Lifetime = param.kind { continue; } From ba8b4a4f824ea1fa1f5e31ab619599be80f14a34 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 14:55:16 -0300 Subject: [PATCH 10/12] Use let else here --- .../src/traits/coherence.rs | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 60111446b946e..6c3ce3ee371b3 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -387,42 +387,42 @@ fn resolve_negative_obligation<'cx, 'tcx>( ) -> bool { let tcx = infcx.tcx; - if let Some(o) = o.flip_polarity(tcx) { - let mut fulfillment_cx = FulfillmentContext::new(); - fulfillment_cx.register_predicate_obligation(infcx, o); + let Some(o) = o.flip_polarity(tcx) else { + return false; + }; - let errors = fulfillment_cx.select_all_or_error(infcx); + let mut fulfillment_cx = FulfillmentContext::new(); + fulfillment_cx.register_predicate_obligation(infcx, o); - if !errors.is_empty() { - return false; - } + let errors = fulfillment_cx.select_all_or_error(infcx); - let mut outlives_env = OutlivesEnvironment::new(param_env); - // FIXME -- add "assumed to be well formed" types into the `outlives_env` + if !errors.is_empty() { + return false; + } - // "Save" the accumulated implied bounds into the outlives environment - // (due to the FIXME above, there aren't any, but this step is still needed). - // The "body id" is given as `CRATE_HIR_ID`, which is the same body-id used - // by the "dummy" causes elsewhere (body-id is only relevant when checking - // function bodies with closures). - outlives_env.save_implied_bounds(CRATE_HIR_ID); + let mut outlives_env = OutlivesEnvironment::new(param_env); + // FIXME -- add "assumed to be well formed" types into the `outlives_env` - infcx.process_registered_region_obligations( - outlives_env.region_bound_pairs_map(), - Some(tcx.lifetimes.re_root_empty), - param_env, - ); + // "Save" the accumulated implied bounds into the outlives environment + // (due to the FIXME above, there aren't any, but this step is still needed). + // The "body id" is given as `CRATE_HIR_ID`, which is the same body-id used + // by the "dummy" causes elsewhere (body-id is only relevant when checking + // function bodies with closures). + outlives_env.save_implied_bounds(CRATE_HIR_ID); - let errors = infcx.resolve_regions(region_context, &outlives_env, RegionckMode::default()); + infcx.process_registered_region_obligations( + outlives_env.region_bound_pairs_map(), + Some(tcx.lifetimes.re_root_empty), + param_env, + ); - if !errors.is_empty() { - return false; - } + let errors = infcx.resolve_regions(region_context, &outlives_env, RegionckMode::default()); - return true; + if !errors.is_empty() { + return false; } - false + true } pub fn trait_ref_is_knowable<'tcx>( From 9e187211199f2697792f3833e7b88fe2a07de828 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 17 Mar 2022 14:27:42 -0400 Subject: [PATCH 11/12] update Miri --- src/tools/miri | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri b/src/tools/miri index 7bc0c98621762..8e818ffa1b85f 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit 7bc0c986217629e6c831edcb133532023a5aec63 +Subproject commit 8e818ffa1b85f4e740c4096fd38c62b2b73f4d83 From 89a00cc8ae92f159325a7b5cb06a6d7802a4f3c5 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 17 Mar 2022 16:51:30 -0400 Subject: [PATCH 12/12] Update compiler/rustc_trait_selection/src/traits/coherence.rs --- compiler/rustc_trait_selection/src/traits/coherence.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 6c3ce3ee371b3..94a4001bbb91a 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -368,7 +368,6 @@ fn negative_impl_exists<'cx, 'tcx>( } // Try to prove a negative obligation exist for super predicates - // FIXME we should implement an `assemble_neg_candidates` fn for o in util::elaborate_predicates(infcx.tcx, iter::once(o.predicate)) { if resolve_negative_obligation(infcx, param_env, region_context, &o) { return true;