Add an `is_effect` flag to `GenericArgKind` #118785

fee1-dead · 2023-12-09T16:33:16Z

Which should help us prevent printing <false> (effects desugaring) in diagnostics.

r? @compiler-errors
cc @oli-obk

rustbot · 2023-12-09T16:33:24Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in need_type_info.rs

cc @lcnr

compiler-errors

I think this approach is pretty invasive, and would rather we don't change every usage site of GenericArg::Const just to handle a printing bug.

I would much rather see the call to path_generic_args take an additional &[ty::GenericParamDef] so it can detect host-ness via that param instead. I think this is possible -- if you don't want to attempt this, please let me know, since I may be able to try it myself.

fee1-dead · 2023-12-09T16:56:23Z

I would much rather see the call to path_generic_args take an additional &[ty::GenericParamDef] so it can detect host-ness via that param instead. I think this is possible -- if you don't want to attempt this, please let me know, since I may be able to try it myself.

Sounds like a good approach. I probably won't have more time to work on this this weekend, so feel free to pick it up if you want.

oli-obk · 2023-12-09T16:57:39Z

Could we add a lang item struct Effect(bool) that we'd use for the type of all effect consts and detect it that way without needing to change a lot?

fee1-dead · 2023-12-09T16:59:46Z

Could we add a lang item struct Effect(bool) that we'd use for the type of all effect consts and detect it that way without needing to change a lot?

hmmm, so we can just see if the defid of the type of the const matches? sounds like a good idea too.

src/librustdoc/clean/utils.rs

            if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = params[index].kind
            {
                return None;
            }



rust-log-analyzer · 2023-12-09T17:15:20Z

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Built container sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Looks like docker image is the same as before, not uploading
https://ci-caches.rust-lang.org/docker/7ebc15c01a233894034d277c8cce4e949f4e7791f66b4727c8fb6e058a0b8171d6152e1441d677cef0653843ceeee469c097b8699b2bb74249e674f6aa1a8813
sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Sat Dec  9 17:00:21 UTC 2023
  network time: Sat, 09 Dec 2023 17:00:21 GMT
  network time: Sat, 09 Dec 2023 17:00:21 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-missing-tools', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: rust.codegen-units-std := 1
---
stdout: none
--- stderr -------------------------------
thread 'rustc' panicked at /checkout/tests/ui-fulldeps/stable-mir/check_defs.rs:50:5:
assertion `left == right` failed
  left: "from_u32::<true>"
 right: "from_u32"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
------------------------------------------

oli-obk · 2023-12-09T17:26:38Z

hmmm, so we can just see if the defid of the type of the const matches? sounds like a good idea too.

kind of, yea. we already have the adtflags that we can check for Box and UnsafeCell, adding another entry there should not be an issue if we have the bits left to do it

…-dead Don't print host effect param in pretty `path_generic_args` Make `own_args_no_defaults` pass back the `GenericParamDef`, so that we can pass both the args *and* param definitions into `path_generic_args`. That allows us to use the `GenericParamDef` to filter out effect params. This allows us to filter out the host param regardless of whether it's `sym::host` or `true`/`false`. This also renames a couple of `const_effect_param` -> `host_effect_param`, and restores `~const` pretty printing to `TraitPredPrintModifiersAndPath`. cc rust-lang#118785 r? `@fee1-dead` cc `@oli-obk`

bors · 2023-12-10T09:53:28Z

☔ The latest upstream changes (presumably #118788) made this pull request unmergeable. Please resolve the merge conflicts.

fee1-dead added 2 commits December 9, 2023 16:31

Add an is_effect flag to GenericArgKind

61813af

fix clippy

7a3559d

fee1-dead marked this pull request as ready for review December 9, 2023 16:33

rustbot assigned compiler-errors Dec 9, 2023

This comment has been minimized.

Sign in to view

compiler-errors requested changes Dec 9, 2023

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2023

fix IR changes after rebase

9950f54

fee1-dead marked this pull request as draft December 9, 2023 17:01

fmease reviewed Dec 9, 2023

View reviewed changes

src/librustdoc/clean/utils.rs

Comment on lines 127 to 131

if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = params[index].kind

{

return None;

}

This comment was marked as outdated.

Sign in to view

compiler-errors mentioned this pull request Dec 9, 2023

Don't print host effect param in pretty path_generic_args #118788

Merged

fee1-dead closed this Dec 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an `is_effect` flag to `GenericArgKind` #118785

Add an `is_effect` flag to `GenericArgKind` #118785

fee1-dead commented Dec 9, 2023 •

edited

Loading

rustbot commented Dec 9, 2023

This comment has been minimized.

compiler-errors left a comment

fee1-dead commented Dec 9, 2023

oli-obk commented Dec 9, 2023

fee1-dead commented Dec 9, 2023

This comment was marked as outdated.

rust-log-analyzer commented Dec 9, 2023

oli-obk commented Dec 9, 2023

bors commented Dec 10, 2023

Add an is_effect flag to GenericArgKind #118785

Add an is_effect flag to GenericArgKind #118785

Conversation

fee1-dead commented Dec 9, 2023 • edited Loading

rustbot commented Dec 9, 2023

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

fee1-dead commented Dec 9, 2023

oli-obk commented Dec 9, 2023

fee1-dead commented Dec 9, 2023

This comment was marked as outdated.

rust-log-analyzer commented Dec 9, 2023

oli-obk commented Dec 9, 2023

bors commented Dec 10, 2023

Add an `is_effect` flag to `GenericArgKind` #118785

Add an `is_effect` flag to `GenericArgKind` #118785

fee1-dead commented Dec 9, 2023 •

edited

Loading