From 81a583c21e74d600ef8c4b45a3d5088382300e17 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Wed, 3 Aug 2022 22:24:47 +0200 Subject: [PATCH 1/2] Try normalizing types without RevealAll in ParamEnv in mir validation Before, the MIR validator used RevealAll in its ParamEnv for type checking. This could cause false negatives in some cases due to RevealAll ParamEnvs not always use all predicates as expected here. Since some MIR passes like inlining use RevealAll as well, keep using it in the MIR validator too, but when it fails usign RevealAll, also try the check without it, to stop false negatives. --- .../src/transform/validate.rs | 28 +++++++++++++------ src/test/ui/mir/issue-99866.rs | 25 +++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/mir/issue-99866.rs diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 45a94972c1134..ddde9ff4c0281 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -181,16 +181,28 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { if (src, dest).has_opaque_types() { return true; } + + let try_equal_with_param_env = |param_env| { + let src = self.tcx.normalize_erasing_regions(param_env, src); + let dest = self.tcx.normalize_erasing_regions(param_env, dest); + // Type-changing assignments can happen when subtyping is used. While + // all normal lifetimes are erased, higher-ranked types with their + // late-bound lifetimes are still around and can lead to type + // differences. So we compare ignoring lifetimes. + equal_up_to_regions(self.tcx, param_env, src, dest) + }; + // Normalize projections and things like that. + // First, try with reveal_all. This might not work in some cases, as the predicates + // can be cleared in reveal_all mode. We try the reveal first anyways as it is used + // by some other passes like inlining as well. let param_env = self.param_env.with_reveal_all_normalized(self.tcx); - let src = self.tcx.normalize_erasing_regions(param_env, src); - let dest = self.tcx.normalize_erasing_regions(param_env, dest); - - // Type-changing assignments can happen when subtyping is used. While - // all normal lifetimes are erased, higher-ranked types with their - // late-bound lifetimes are still around and can lead to type - // differences. So we compare ignoring lifetimes. - equal_up_to_regions(self.tcx, param_env, src, dest) + if try_equal_with_param_env(param_env) { + true + } else { + // If this fails, we can try it without the reveal. + try_equal_with_param_env(self.param_env) + } } } diff --git a/src/test/ui/mir/issue-99866.rs b/src/test/ui/mir/issue-99866.rs new file mode 100644 index 0000000000000..d39ae6ebf1da2 --- /dev/null +++ b/src/test/ui/mir/issue-99866.rs @@ -0,0 +1,25 @@ +// check-pass +pub trait Backend { + type DescriptorSetLayout; +} + +pub struct Back; + +impl Backend for Back { + type DescriptorSetLayout = u32; +} + +pub struct HalSetLayouts { + vertex_layout: ::DescriptorSetLayout, +} + +impl HalSetLayouts { + pub fn iter(self) -> DSL + where + Back: Backend, + { + self.vertex_layout + } +} + +fn main() {} From 96d4137deed6c52c6db2dd19568c37d1c160f1e7 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sat, 6 Aug 2022 14:22:57 +0200 Subject: [PATCH 2/2] Only normalize once in mir validator typechecker Before, it called `normalize_erasing_regions` twice since `equal_up_to_regions` called it as well for both types. --- .../src/transform/validate.rs | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index ddde9ff4c0281..69113e57bdc5b 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -182,27 +182,22 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return true; } - let try_equal_with_param_env = |param_env| { - let src = self.tcx.normalize_erasing_regions(param_env, src); - let dest = self.tcx.normalize_erasing_regions(param_env, dest); - // Type-changing assignments can happen when subtyping is used. While - // all normal lifetimes are erased, higher-ranked types with their - // late-bound lifetimes are still around and can lead to type - // differences. So we compare ignoring lifetimes. - equal_up_to_regions(self.tcx, param_env, src, dest) - }; - // Normalize projections and things like that. + // Type-changing assignments can happen when subtyping is used. While + // all normal lifetimes are erased, higher-ranked types with their + // late-bound lifetimes are still around and can lead to type + // differences. So we compare ignoring lifetimes. + // First, try with reveal_all. This might not work in some cases, as the predicates // can be cleared in reveal_all mode. We try the reveal first anyways as it is used // by some other passes like inlining as well. let param_env = self.param_env.with_reveal_all_normalized(self.tcx); - if try_equal_with_param_env(param_env) { - true - } else { - // If this fails, we can try it without the reveal. - try_equal_with_param_env(self.param_env) + if equal_up_to_regions(self.tcx, param_env, src, dest) { + return true; } + + // If this fails, we can try it without the reveal. + equal_up_to_regions(self.tcx, self.param_env, src, dest) } }