Skip to content

Add and use generics.is_empty(), rather than checking generics' params vec #123929

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

Closed
wants to merge 2 commits into from

Conversation

compiler-errors
Copy link
Member

r? spastorino

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

HIR ty lowering was modified

cc @fmease

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Copy link
Member Author

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, after a self-review it's kind of certain there are more places where it's wrong to use this than right.

@@ -430,7 +430,7 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, trait_def_id: LocalDefId) {
}
let gat_generics = tcx.generics_of(gat_def_id);
// FIXME(jackh726): we can also warn in the more general case
if gat_generics.params.is_empty() {
if gat_generics.is_empty() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this is wrong!

@@ -450,7 +450,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
// `foo.bar::<u32>(...)` -- the `Self` type here will be the
// type of `foo` (possibly adjusted), but we don't want to
// include that. We want just the `[_, u32]` part.
if !args.is_empty() && !generics.params.is_empty() {
if !args.is_empty() && !generics.is_empty() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also wrong

@@ -411,7 +411,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// Traits always have `Self` as a generic parameter, which means they will not return early
// here and so associated type bindings will be handled regardless of whether there are any
// non-`Self` generic parameters.
if generics.params.is_empty() {
if generics.is_empty() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong

@@ -282,7 +282,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if !self_ty_name.contains('<') {
if let Adt(def, _) = self_ty.kind() {
let generics = self.tcx.generics_of(def.did());
if !generics.params.is_empty() {
if !generics.is_empty() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong

@@ -1871,7 +1871,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
let generics = self.tcx.generics_of(method);
assert_eq!(args.len(), generics.parent_count);

let xform_fn_sig = if generics.params.is_empty() {
let xform_fn_sig = if generics.is_empty() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong

@@ -157,7 +157,7 @@ pub trait Printer<'tcx>: Sized {
// If we have any generic arguments to print, we do that
// on top of the same path, but without its own generics.
_ => {
if !generics.params.is_empty() && args.len() >= generics.count() {
if !generics.is_empty() && args.len() >= generics.count() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong

@@ -397,7 +397,7 @@ pub fn object_safety_violations_for_assoc_item(
// Associated types can only be object safe if they have `Self: Sized` bounds.
ty::AssocKind::Type => {
if !tcx.features().generic_associated_types_extended
&& !tcx.generics_of(item.def_id).params.is_empty()
&& !tcx.generics_of(item.def_id).is_empty()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong

@@ -1773,7 +1773,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// If this type is a GAT, and of the GAT args resolve to something new,
// that means that we must have newly inferred something about the GAT.
// We should give up in that case.
if !generics.params.is_empty()
if !generics.is_empty()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong

@@ -225,7 +225,7 @@ impl {self_ty_without_ref} {{
&& let ImplItemKind::Fn(sig, _) = item.kind
&& let FnRetTy::Return(ret) = sig.decl.output
&& is_nameable_in_impl_trait(ret)
&& cx.tcx.generics_of(item_did).params.is_empty()
&& cx.tcx.generics_of(item_did).is_empty()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is wrong

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
#16 exporting to docker image format
#16 sending tarball 29.2s done
#16 DONE 31.7s
##[endgroup]
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-17]
---
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-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--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-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
   Compiling compiler_builtins v0.1.108
error[E0038]: the trait `iterator::Iterator` cannot be made into an object
  --> library/core/src/iter/traits/iterator.rs:17:31
   |
17 | fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
   |                               ^^^^^^^^^^^^^^^^^^^^^^^ `iterator::Iterator` cannot be made into an object
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
  --> library/core/src/iter/traits/iterator.rs:48:10
   |
44 | pub trait Iterator {
44 | pub trait Iterator {
   |           -------- this trait cannot be made into an object...
...
48 |     type Item;
   |          ^^^^ ...because it contains the generic associated type `Item`
   = help: consider moving `Item` to another trait
   = note: `iterator::Iterator` can be implemented in other crates; if you want to support your users passing their own types here, you can't refer to a specific type
error: rustc interrupted by SIGSEGV, printing backtrace


/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0xb78636)[0x7fcd1dcbf636]
/lib/x86_64-linux-gnu/libc.so.6(+0x42990)[0x7fcd1ce27990]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3ac0734)[0x7fcd20c07734]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(_RNvMs0_NtNtCs5SMM7wDgPVu_12rustc_middle2ty8genericsNtB5_8Generics20own_args_no_defaults+0xc2)[0x7fcd20cdcc32]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aba59f)[0x7fcd20c0159f]
### cycle encountered after 5 frames with period 6
### cycle encountered after 5 frames with period 6
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(_RNvXs4_NtNtNtCs5SMM7wDgPVu_12rustc_middle2ty5print6prettyNtB5_10FmtPrinterNtB7_7Printer14print_def_path+0x1a1)[0x7fcd20bf2531]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aac06f)[0x7fcd20bf306f]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aba5cb)[0x7fcd20c015cb]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(_RNvXs4_NtNtNtCs5SMM7wDgPVu_12rustc_middle2ty5print6prettyNtB5_10FmtPrinterNtB7_7Printer14print_def_path+0x1a1)[0x7fcd20bf2531]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aac06f)[0x7fcd20bf306f]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aba5cb)[0x7fcd20c015cb]
### recursed 41 times

/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(_RNvXs4_NtNtNtCs5SMM7wDgPVu_12rustc_middle2ty5print6prettyNtB5_10FmtPrinterNtB7_7Printer14print_def_path+0x1a1)[0x7fcd20bf2531]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aac06f)[0x7fcd20bf306f]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aba5cb)[0x7fcd20c015cb]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(_RNvXs4_NtNtNtCs5SMM7wDgPVu_12rustc_middle2ty5print6prettyNtB5_10FmtPrinterNtB7_7Printer14print_def_path+0x1a1)[0x7fcd20bf2531]
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib/librustc_driver-a3dc4824b0f0d511.so(+0x3aac06f)[0x7fcd20bf306f]

note: rustc unexpectedly overflowed its stack! this is a bug
note: maximum backtrace depth reached, frames may have been lost
note: we would appreciate a report at https://github.com/rust-lang/rust
help: you can increase rustc's stack size by setting RUST_MIN_STACK=16777216
note: backtrace dumped due to SIGSEGV! resuming signal
rustc exited with signal: 11 (SIGSEGV) (core dumped)

Caused by:
Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc /checkout/obj/build/bootstrap/debug/rustc --crate-name core --edition=2021 library/core/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C embed-bitcode=no -C codegen-units=1 -C debug-assertions=on -Zunstable-options --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values("debug_refcell", "panic_immediate_abort"))' -C metadata=70719e8645e6f000 -C extra-filename=-70719e8645e6f000 --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps -Csymbol-mangling-version=v0 '--check-cfg=cfg(feature,values(any()))' -Zunstable-options '--check-cfg=cfg(bootstrap)' '--check-cfg=cfg(stdarch_intel_sde)' '--check-cfg=cfg(no_fp_fmt_parse)' '--check-cfg=cfg(no_global_oom_handling)' '--check-cfg=cfg(no_rc)' '--check-cfg=cfg(no_sync)' '--check-cfg=cfg(netbsd10)' '--check-cfg=cfg(backtrace_in_libstd)' '--check-cfg=cfg(target_env,values("libnx","p2"))' '--check-cfg=cfg(target_os,values("visionos"))' '--check-cfg=cfg(target_arch,values("arm64ec","spirv","nvptx","xtensa"))' -Zmacro-backtrace -Csplit-debuginfo=off -Cprefer-dynamic -Cllvm-args=-import-instr-limit=10 -Zinline-mir -Clink-args=-Wl,-z,origin '-Clink-args=-Wl,-rpath,$ORIGIN/../lib' -Cembed-bitcode=yes -Cforce-frame-pointers=yes '-Zcrate-attr=doc(html_root_url="https://doc.rust-lang.org/nightly/")' -Z binary-dep-depinfo` (exit status: 254)
  local time: Sun Apr 14 15:14:40 UTC 2024
  network time: Sun, 14 Apr 2024 15:14:40 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@spastorino
Copy link
Member

I guess you have decided to not proceed with this change, right?. I was suggesting something like an is_empty and an is_own_empty or something like that where you have both functions and we stop doing .params.is_empty and/or using count.

@compiler-errors
Copy link
Member Author

@spastorino: feel free to do the change yourself -- you can use the comments above to figure out where we care about is_own_empty vs is_empty.

bors added a commit to rust-lang-ci/rust that referenced this pull request May 19, 2024
…iler-errors

Add and use generics.is_empty() and generics.is_own_empty, rather than using generics' attributes

r? `@compiler-errors`

Related to rust-lang#123929
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants