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

Implement RFC 3624 supertrait_item_shadowing (v2) #125782

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented May 30, 2024

Implements RFC 3624 and the associated lint in the RFC.

Implements:

  • Shadowing algorithm
  • Lint for call-site shadowing (allow by default, gated)
  • Lint for definition-site shadowing (allow by default, gated)

Tracking:

cc @Amanieu and rust-lang/rfcs#3624 and #89151

@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 May 30, 2024
@compiler-errors compiler-errors changed the title Implement RFC 3624 supertrait_item_shadowing Implement RFC 3624 supertrait_item_shadowing (v2) May 30, 2024
@traviscross traviscross added the F-supertrait_item_shadowing `#![feature(supertrait_item_shadowing)]` label May 30, 2024
@bors
Copy link
Contributor

bors commented Jul 29, 2024

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

@Dylan-DPC Dylan-DPC added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 17, 2024
@compiler-errors compiler-errors force-pushed the supertrait-item-shadowing branch 3 times, most recently from e95db1b to 8665a82 Compare September 19, 2024 01:39
@compiler-errors
Copy link
Member Author

OK, this is ready pending RFC approval.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors compiler-errors force-pushed the supertrait-item-shadowing branch from 8665a82 to d03f75d Compare September 19, 2024 01:48
@compiler-errors compiler-errors marked this pull request as ready for review September 19, 2024 15:51
@compiler-errors
Copy link
Member Author

The RFC is now in FCP, so this could probably get reviewed preliminarily but should probably not land until the RFC is merged.

r? compiler

@bors
Copy link
Contributor

bors commented Sep 20, 2024

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 20, 2024
@compiler-errors compiler-errors force-pushed the supertrait-item-shadowing branch from d03f75d to ffbb443 Compare October 11, 2024 08:26
@compiler-errors compiler-errors force-pushed the supertrait-item-shadowing branch from ffbb443 to 1ab7845 Compare November 21, 2024 01:42
@rust-log-analyzer

This comment has been minimized.

@compiler-errors compiler-errors force-pushed the supertrait-item-shadowing branch from 1ab7845 to e59cf24 Compare November 21, 2024 04:00
@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

I think there’s a few missing words in the lint comments, and maybe some places that say “method” should say “item”?

compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/messages.ftl Outdated Show resolved Hide resolved
@compiler-errors compiler-errors force-pushed the supertrait-item-shadowing branch from e59cf24 to 8409aaa Compare January 25, 2025 17:30
@compiler-errors
Copy link
Member Author

@rustbot ready

This PR is ready for review since the corresponding RFC has landed.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 25, 2025
@compiler-errors compiler-errors removed the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Jan 25, 2025
@compiler-errors
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 25, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2025
…owing, r=<try>

Implement RFC 3624 `supertrait_item_shadowing` (v2)

Implements RFC 3624 and the associated lint in the RFC.

Implements:
* Shadowing algorithm
* Lint for call-site shadowing (allow by default, gated)
* Lint for definition-site shadowing (allow by default, gated)

Tracking:
- rust-lang#89151

cc `@Amanieu` and rust-lang/rfcs#3624 and rust-lang#89151
@bors
Copy link
Contributor

bors commented Jan 25, 2025

⌛ Trying commit 8409aaa with merge 2f0cace...

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
#21 exporting to docker image format
#21 sending tarball 27.8s done
#21 DONE 41.3s
##[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-18]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
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-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--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', '--set', 'rust.lld=false', '--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-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
  Downloaded boml v0.3.1
   Compiling boml v0.3.1
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished `release` profile [optimized] target(s) in 3.96s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --mini-tests --std-tests`
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
---

failures:

---- compiler/rustc_lint_defs/src/builtin.rs - builtin::SUPERTRAIT_ITEM_SHADOWING_DEFINITION (line 5019) stdout ----
error: trait item `hello` from `Downstream` shadows identically named item from supertrait
   |
12 |     fn hello(&self) {}
   |     ^^^^^^^^^^^^^^^
   |
---
error: aborting due to 1 previous error

Couldn't compile the test.
---- compiler/rustc_lint_defs/src/builtin.rs - builtin::SUPERTRAIT_ITEM_SHADOWING_USAGE (line 4977) stdout ----
error: trait item `hello` from `Downstream` shadows identically named item from supertrait
   |
   |
17 | MyType.hello();
   |
note: item from `Downstream` shadows a supertrait item
  --> compiler/rustc_lint_defs/src/builtin.rs:4987:5
   |
---
    compiler/rustc_lint_defs/src/builtin.rs - builtin::SUPERTRAIT_ITEM_SHADOWING_USAGE (line 4977)

test result: FAILED. 118 passed; 2 failed; 25 ignored; 0 measured; 0 filtered out; finished in 1.15s

error: doctest failed, to rerun pass `-p rustc_lint_defs --doc`
  local time: Sat Jan 25 18:05:01 UTC 2025
  network time: Sat, 25 Jan 2025 18:05:01 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Jan 25, 2025

☀️ Try build successful - checks-actions
Build commit: 2f0cace (2f0cacef566fb90f013a70c8152953b70aca415d)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2f0cace): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.3% [0.1%, 3.7%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 2
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.5%, secondary 3.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.3% [3.0%, 3.6%] 2
Improvements ✅
(primary)
-1.5% [-2.0%, -1.1%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.5% [-2.0%, -1.1%] 2

Cycles

Results (secondary 3.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.0% [3.0%, 3.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 770.579s -> 771.216s (0.08%)
Artifact size: 325.79 MiB -> 325.83 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-supertrait_item_shadowing `#![feature(supertrait_item_shadowing)]` 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.

9 participants