From 89308bbd1a65e7ff2523215a4665e5bbda9a5188 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 5 Jan 2020 22:32:53 -0500 Subject: [PATCH 1/5] Don't run const propagation on items with inconsistent bounds Using `#![feature(trivial_bounds)]`, it's possible to write functions with unsatisfiable 'where' clauses, making them uncallable. However, the user can act as if these 'where' clauses are true inside the body of the function, leading to code that would normally be impossible to write. Since const propgation can run even without any user-written calls to a function, we need to explcitly check for these uncallable functions. --- src/librustc_mir/transform/const_prop.rs | 25 +++++++++++++++++++ ...ounds-inconsistent-associated-functions.rs | 4 +++ ...s-inconsistent-associated-functions.stderr | 2 ++ 3 files changed, 31 insertions(+) create mode 100644 src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 31c3a2c7ebceb..7a795329a0b66 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -75,6 +75,31 @@ impl<'tcx> MirPass<'tcx> for ConstProp { return; } + // Check if it's even possible to satisy the 'where' clauses + // for this item. + // This branch will never be taken for any normal function. + // However, it's possible to `#!feature(trivial_bounds)]` to write + // a function with impossible to satisfy clauses, e.g.: + // `fn foo() where String: Copy {}` + // + // We don't usually need to worry about this kind of case, + // since we would get a compilation error if the user tried + // to call it. However, since we can do const propagation + // even without any calls to the function, we need to make + // sure that it even makes sense to try to evaluate the body. + // If there are unsatisfiable where clauses, then all bets are + // off, and we just give up. + if !tcx.substitute_normalize_and_test_predicates(( + source.def_id(), + InternalSubsts::identity_for_item(tcx, source.def_id()), + )) { + trace!( + "ConstProp skipped for item with unsatisfiable predicates: {:?}", + source.def_id() + ); + return; + } + trace!("ConstProp starting for {:?}", source.def_id()); let dummy_body = &Body::new( diff --git a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs index 6450ddd1b67fc..818be10554720 100644 --- a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs +++ b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs @@ -1,4 +1,8 @@ // run-pass +// Force mir to be emitted, to ensure that const +// propagation doesn't ICE on a function +// with an 'impossible' body. See issue #67696 +// compile-flags: --emit=mir,link // Inconsistent bounds with trait implementations #![feature(trivial_bounds)] diff --git a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr new file mode 100644 index 0000000000000..0ee1e2e0ba56a --- /dev/null +++ b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr @@ -0,0 +1,2 @@ +warning: due to multiple output types requested, the explicitly specified output file name will be adapted for each output type + From 4a84027f21d1806d3ae62665f14119f2c249caba Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 7 Jan 2020 10:01:52 -0500 Subject: [PATCH 2/5] Add additional regression test --- .../ui/consts/issue-67696-const-prop-ice.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 src/test/ui/consts/issue-67696-const-prop-ice.rs diff --git a/src/test/ui/consts/issue-67696-const-prop-ice.rs b/src/test/ui/consts/issue-67696-const-prop-ice.rs new file mode 100644 index 0000000000000..ad52608b3f46d --- /dev/null +++ b/src/test/ui/consts/issue-67696-const-prop-ice.rs @@ -0,0 +1,20 @@ +// check-pass +// compile-flags: --emit=mir,link +// Checks that we don't ICE due to attempting to run const prop +// on a function with unsatisifable 'where' clauses + +#![allow(unused)] + +trait A { + fn foo(&self) -> Self where Self: Copy; +} + +impl A for [fn(&())] { + fn foo(&self) -> Self where Self: Copy { *(&[] as &[_]) } +} + +impl A for i32 { + fn foo(&self) -> Self { 3 } +} + +fn main() {} From 9951450e8bead779bb7b209c8f884916440525a2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 7 Jan 2020 10:53:04 -0500 Subject: [PATCH 3/5] Fix typo --- src/librustc_mir/transform/const_prop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7a795329a0b66..012217dba8b70 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -75,7 +75,7 @@ impl<'tcx> MirPass<'tcx> for ConstProp { return; } - // Check if it's even possible to satisy the 'where' clauses + // Check if it's even possible to satisfy the 'where' clauses // for this item. // This branch will never be taken for any normal function. // However, it's possible to `#!feature(trivial_bounds)]` to write From 7f26d32cd61e9e8a3029e269b18ddccb5a376c43 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 11 Jan 2020 13:20:09 -0500 Subject: [PATCH 4/5] Convert test to check-pass --- .../trivial-bounds-inconsistent-associated-functions.rs | 4 ++-- .../trivial-bounds-inconsistent-associated-functions.stderr | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) delete mode 100644 src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr diff --git a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs index 818be10554720..e0d4d538b40e7 100644 --- a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs +++ b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs @@ -1,8 +1,8 @@ -// run-pass +// check-pass +// compile-flags: --emit=mir // Force mir to be emitted, to ensure that const // propagation doesn't ICE on a function // with an 'impossible' body. See issue #67696 -// compile-flags: --emit=mir,link // Inconsistent bounds with trait implementations #![feature(trivial_bounds)] diff --git a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr deleted file mode 100644 index 0ee1e2e0ba56a..0000000000000 --- a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.stderr +++ /dev/null @@ -1,2 +0,0 @@ -warning: due to multiple output types requested, the explicitly specified output file name will be adapted for each output type - From b4125f003de6bb0d766038fc366da0c79d623c4f Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 11 Jan 2020 14:17:42 -0500 Subject: [PATCH 5/5] Add "--emit=link" This avoids a strange linker error that we get with only "--emit=mir" and "check-pass" --- .../trivial-bounds-inconsistent-associated-functions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs index e0d4d538b40e7..69eee66e64d89 100644 --- a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs +++ b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs @@ -1,5 +1,5 @@ // check-pass -// compile-flags: --emit=mir +// compile-flags: --emit=mir,link // Force mir to be emitted, to ensure that const // propagation doesn't ICE on a function // with an 'impossible' body. See issue #67696