Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalization can skip WF #100041

Open
CAD97 opened this issue Aug 2, 2022 · 11 comments
Open

Normalization can skip WF #100041

CAD97 opened this issue Aug 2, 2022 · 11 comments
Labels
A-associated-items Area: Associated items (types, constants & functions) C-bug Category: This is a bug. fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@CAD97
Copy link
Contributor

CAD97 commented Aug 2, 2022

I tried this code: [playground]

trait WellUnformed {
    type RequestNormalize;
}

impl<T: ?Sized> WellUnformed for T {
    type RequestNormalize = ();
}

const _: <[[[[[[u8]]]]]] as WellUnformed>::RequestNormalize = ();
const _: <Vec<str> as WellUnformed>::RequestNormalize = ();

I expected this to error; [[[[[[u8]]]]]] is clearly not a well-formed type, as

error[E0277]: the size for values of type [u8] cannot be known at compilation time
= help: the trait Sized is not implemented for [u8]
= note: slice and array elements must have Sized type

Vec<str> is perhaps even more cursed.

Instead: it compiles.

This is especially relevant for blanket/builtin traits which have associated types, such as marker::DiscriminantKind and ptr::Pointee.

Meta

1.64.0-nightly (2022-07-31 f9cba63)

@rustbot label +A-associated-items

cc @eddyb who said this was
@rustbot label +I-unsound

@CAD97 CAD97 added the C-bug Category: This is a bug. label Aug 2, 2022
@rustbot rustbot added A-associated-items Area: Associated items (types, constants & functions) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 2, 2022
@eddyb
Copy link
Member

eddyb commented Aug 2, 2022

cc @rust-lang/types

@CAD97
Copy link
Contributor Author

CAD97 commented Aug 2, 2022

@rustbot label +T-types

@rustbot rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Aug 2, 2022
@compiler-errors
Copy link
Member

compiler-errors commented Aug 2, 2022

I think I found the bug(s). This is:

  1. partly due to Try to normalize associated types before processing obligations #90887, which normalizes WellFormed predicates before we can actually walk through the projection type and register a WF obligation for the projection's self ty,
  2. and partly due to us doing a normalize call before registering a WellFormed predicate in wfcheck code.

Let me put up a PR.

@jackh726
Copy link
Member

jackh726 commented Aug 2, 2022

Is this fixed in #99217?

@compiler-errors
Copy link
Member

Just checked out that PR, and it's not fixed by #99217.

@apiraino
Copy link
Contributor

apiraino commented Aug 3, 2022

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 3, 2022
@CAD97
Copy link
Contributor Author

CAD97 commented Sep 19, 2022

Status: still happens. #100046 is blocked on implied bounds for unnormalized types.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 23, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? `@jackh726`

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? ``@jackh726``

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? `@jackh726`

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? ``@jackh726``

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? `@jackh726`

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
JohnTitor added a commit to JohnTitor/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? ``@jackh726``

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
@jackh726 jackh726 added the S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. label Apr 25, 2023
@CAD97
Copy link
Contributor Author

CAD97 commented Jun 12, 2023

@rustbot label +I-ICE

I turned this into an ICE:

pub trait WellUnformed {
    type RequestNormalize;
}

impl<T: ?Sized> WellUnformed for T {
    type RequestNormalize = ();
}

pub fn latent(_: &[<[[()]] as WellUnformed>::RequestNormalize; 0]) {}

pub fn bang() {
    latent(&[]);
}
error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: error performing ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: UserFacing, constness: NotConst }, value: ProvePredicate { predicate: Binder(WellFormed(&[<[[()]] as WellUnformed>::RequestNormalize; 0]), []) } }
  --> src\lib.rs:12:5
   |
12 |     latent(&[]);
   |     ^^^^^^^^^^^
   |
   = note: delayed at /rustc/7820972f866ca2cea281ccc39201c0cd27087110\compiler\rustc_trait_selection\src\traits\query\type_op\mod.rs:162:32
              0: std::backtrace::Backtrace::disabled
              1: std::backtrace::Backtrace::capture
              2: <rustc_errors::HandlerInner>::emit_diagnostic
              3: <rustc_borrowck::type_check::InstantiateOpaqueType as core::fmt::Debug>::fmt
              4: <rustc_borrowck::region_infer::values::PointIndex as core::ops::arith::Add<usize>>::add
              5: <rustc_borrowck::type_check::Locations>::span
              6: <rustc_middle::mir::syntax::Place as rustc_borrowck::place_ext::PlaceExt>::ignore_borrow
              7: <rustc_borrowck::constraints::OutlivesConstraintSet as core::ops::index::Index<rustc_borrowck::constraints::OutlivesConstraintIndex>>::index
              8: <rustc_borrowck::MirBorrowckCtxt>::consume_operand
              9: <rustc_middle::ty::adjustment::AutoBorrowMutability as rustc_mir_build::thir::cx::expr::ToBorrowKind>::to_borrow_kind
             10: rustc_query_impl::query_callbacks
             11: rustc_query_impl::query_callbacks
             12: rustc_query_impl::query_callbacks
             13: rustc_query_impl::profiling_support::alloc_self_profile_query_strings
             14: rustc_driver_impl::args::arg_expand_all
             15: rustc_interface::interface::parse_check_cfg
             16: rustc_interface::passes::analysis
             17: rustc_query_impl::profiling_support::alloc_self_profile_query_strings
             18: <tracing_subscriber::filter::directive::StaticDirective as core::default::Default>::default
             19: rustc_query_impl::profiling_support::alloc_self_profile_query_strings
             20: rustc_query_impl::profiling_support::alloc_self_profile_query_strings
             21: rustc_query_impl::profiling_support::alloc_self_profile_query_strings
             22: rustc_driver_impl::main
             23: rustc_driver_impl::main
             24: rustc_driver_impl::main
             25: rustc_driver_impl::main
             26: rustc_driver_impl::main
             27: std::sys::windows::thread::Thread::new
             28: BaseThreadInitThunk
             29: RtlUserThreadStart
note: rustc 1.72.0-nightly (7820972f8 2023-06-10) running on x86_64-pc-windows-msvc

Strangely enough, fn latent isn't enough to trigger an ICE; you need to call it. The error happens on cargo check and is pre-mono.

@rustbot rustbot added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Jun 12, 2023
@compiler-errors
Copy link
Member

lol, given the ICE message I thought it was a side-effect of #111918, but it ICEs on stable as well. --- well, better an ICE than unsoundness, eh?

@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Jul 14, 2023
@lcnr lcnr moved this to blocked on new solver in T-types unsound issues Nov 15, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 17, 2024
…-rendering, r=GuillaumeGomez

rustdoc: fix and refactor HTML rendering a bit

* refactoring: get rid of a bunch of manual `f.alternate()` branches
  * not sure why this wasn't done so already, is this perf-sensitive?
* fix an ICE in debug builds of rustdoc
  * rustdoc used to crash on empty outlives-bounds: `where 'a:`
* properly escape const generic defaults
* actually print empty trait and outlives-bounds (doesn't work for cross-crate reexports yet, will fix that at some other point) since they can have semantic significance
  * outlives-bounds: forces lifetime params to be early-bound instead of late-bound which is technically speaking part of the public API
  * trait-bounds: can affect the well-formedness, consider
    * makeshift “const-evaluatable” bounds under `generic_const_exprs`
    * bounds to force wf-checking in light of rust-lang#100041 (quite artificial I know, I couldn't figure out something better), see rust-lang#121160 (comment)
fmease added a commit to fmease/rust that referenced this issue Feb 18, 2024
…-rendering, r=GuillaumeGomez

rustdoc: fix and refactor HTML rendering a bit

* refactoring: get rid of a bunch of manual `f.alternate()` branches
  * not sure why this wasn't done so already, is this perf-sensitive?
* fix an ICE in debug builds of rustdoc
  * rustdoc used to crash on empty outlives-bounds: `where 'a:`
* properly escape const generic defaults
* actually print empty trait and outlives-bounds (doesn't work for cross-crate reexports yet, will fix that at some other point) since they can have semantic significance
  * outlives-bounds: forces lifetime params to be early-bound instead of late-bound which is technically speaking part of the public API
  * trait-bounds: can affect the well-formedness, consider
    * makeshift “const-evaluatable” bounds under `generic_const_exprs`
    * bounds to force wf-checking in light of rust-lang#100041 (quite artificial I know, I couldn't figure out something better), see rust-lang#121160 (comment)
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 18, 2024
Rollup merge of rust-lang#121160 - fmease:rustdoc-fix-n-refactor-html-rendering, r=GuillaumeGomez

rustdoc: fix and refactor HTML rendering a bit

* refactoring: get rid of a bunch of manual `f.alternate()` branches
  * not sure why this wasn't done so already, is this perf-sensitive?
* fix an ICE in debug builds of rustdoc
  * rustdoc used to crash on empty outlives-bounds: `where 'a:`
* properly escape const generic defaults
* actually print empty trait and outlives-bounds (doesn't work for cross-crate reexports yet, will fix that at some other point) since they can have semantic significance
  * outlives-bounds: forces lifetime params to be early-bound instead of late-bound which is technically speaking part of the public API
  * trait-bounds: can affect the well-formedness, consider
    * makeshift “const-evaluatable” bounds under `generic_const_exprs`
    * bounds to force wf-checking in light of rust-lang#100041 (quite artificial I know, I couldn't figure out something better), see rust-lang#121160 (comment)
@lcnr
Copy link
Contributor

lcnr commented Mar 11, 2024

This is currently mostly fixable, except for types in impl headers.

As stated by @aliemjay in #100046 (comment), we can check unnormalized types for WF if the type is from an item signature.

We already use their unnormalized type when computing WF in borrowck, so we can first fix #105948 by using the unnormalized types from fn sigs.

We can then check the wf of function signatures without normalizing them.

@lcnr lcnr added P-medium Medium priority and removed P-high High priority labels May 2, 2024
@lcnr
Copy link
Contributor

lcnr commented May 2, 2024

lowering this from P-high to P-medium as this cannot directly result in undefined runtime behavior by itself. Fixing this will be straightforward once we've imnplemented the better approach to handling implied bounds on binders, which is still a high priority as it also blocks other issues, such as #25860 and #84591.

@fmease fmease added the fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. label Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) C-bug Category: This is a bug. fixed-by-next-solver Fixed by the next-generation trait solver, `-Znext-solver`. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-medium Medium priority S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
Status: new solver everywhere
Development

Successfully merging a pull request may close this issue.

9 participants