-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Warn (or error) when Self
ctor from outer item is referenced in inner nested item
#124187
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
This comment has been minimized.
This comment has been minimized.
r? compiler |
a772589
to
1276a2e
Compare
This comment has been minimized.
This comment has been minimized.
1276a2e
to
e26ae7b
Compare
This comment has been minimized.
This comment has been minimized.
anyways ill fix ci but i guess it's worth testing for regressions @bors try |
This comment was marked as off-topic.
This comment was marked as off-topic.
Warn (or error) when `Self` ctor from outer item is referenced in inner nested item This implements a warning `SELF_CONSTRUCTOR_FROM_OUTER_ITEM` when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned rust-lang#117246 (comment). This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically *never* correct to do. This also reverts part of rust-lang#117246, since I believe this is the proper fix and we shouldn't need the helper functions (`opt_param_at`/`opt_type_param`) any longer, since they shouldn't really ever be used in cases where we don't have this problem.
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
Finished benchmarking commit (773c42f): comparison URL. Overall result: ❌✅ regressions and improvements - no action neededBenchmarking 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 Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 671.572s -> 673.451s (0.28%) |
e26ae7b
to
1c82237
Compare
r? compiler |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
1c82237
to
b86427e
Compare
@rustbot ready |
…explain`" This mostly reverts commit 7449478. It also removes an `opt_param_at` that really is unnecessary given our ICE policy for malformed intrinsics.
b86427e
to
a25bb5f
Compare
@bors r+ |
…enkov Warn (or error) when `Self` ctor from outer item is referenced in inner nested item This implements a warning `SELF_CONSTRUCTOR_FROM_OUTER_ITEM` when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned rust-lang#117246 (comment). This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically *never* correct to do. This also reverts part of rust-lang#117246, since I believe this is the proper fix and we shouldn't need the helper functions (`opt_param_at`/`opt_type_param`) any longer, since they shouldn't really ever be used in cases where we don't have this problem.
A job failed! Check out the build log: (web) (plain) Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (21e6de7): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 672.177s -> 672.624s (0.07%) |
This implements a warning
SELF_CONSTRUCTOR_FROM_OUTER_ITEM
when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned #117246 (comment).This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically never correct to do.
This also reverts part of #117246, since I believe this is the proper fix and we shouldn't need the helper functions (
opt_param_at
/opt_type_param
) any longer, since they shouldn't really ever be used in cases where we don't have this problem.