-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Detect struct construction with private field in field with default #135846
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
Conversation
tests/ui/structs/default-field-values/non-exhaustive-ctor-2.stderr
Outdated
Show resolved
Hide resolved
5f24487
to
7e07e4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preliminary review.
tests/ui/structs/default-field-values/non-exhaustive-ctor-2.stderr
Outdated
Show resolved
Hide resolved
@@ -1022,6 +1023,7 @@ pub struct Resolver<'ra, 'tcx> { | |||
|
|||
/// N.B., this is used only for better diagnostics, not name resolution itself. | |||
field_names: LocalDefIdMap<Vec<Ident>>, | |||
field_defaults: LocalDefIdMap<Vec<Symbol>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Symbol
instead Ident
raises a few warning flags in my head because I'm pretty sure that this means anything involving field_defaults
doesn't handle macro hygiene correctly. This is only relevant to decl macros 2.0 (feature decl_macro
).
It might be a non-issue, I haven't thought that hard yet. I'll need to read the rest of this PR first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worst case it's just wrong diagnostics in macro edge cases
Checking perf due to additional query feeding in the happy path (if I saw that correctly). @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…r=<try> Detect struct construction with private field in field with default When trying to construct a struct that has a public field of a private type, suggest using `..` if that field has a default value. ``` error[E0603]: struct `Priv1` is private --> $DIR/non-exhaustive-ctor.rs:25:39 | LL | let _ = S { field: (), field1: m::Priv1 {} }; | ------ ^^^^^ private struct | | | while setting this field | note: the struct `Priv1` is defined here --> $DIR/non-exhaustive-ctor.rs:14:4 | LL | struct Priv1 {} | ^^^^^^^^^^^^ help: the field `field1` you're trying to set has a default value, you can use `..` to use it | LL | let _ = S { field: (), .. }; | ~~ ```
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (5e87c8e): 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 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.
Max RSS (memory usage)Results (primary 2.1%, secondary -0.1%)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.
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: 770.579s -> 772.7s (0.28%) |
7e07e4f
to
38f8f98
Compare
38f8f98
to
7d576f6
Compare
☔ The latest upstream changes (presumably #128440) made this pull request unmergeable. Please resolve the merge conflicts. |
Hi @fmease, I think this is just waiting on some feedback from you whether this is an issue or not and then a rebase from the author since perf looks neutral. Thanks! |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
I'm not quite sure how this PR affects https://github.com/rust-lang/rust/blob/75e9b19d78a601d07df4d8f19f72165026f32103/tests/debuginfo/lexical-scope-in-if-let.rs at all :( |
c41fee3
to
217048f
Compare
☔ The latest upstream changes (presumably #145077) made this pull request unmergeable. Please resolve the merge conflicts. |
When trying to construct a struct that has a public field of a private type, suggest using `..` if that field has a default value. ``` error[E0603]: struct `Priv1` is private --> $DIR/non-exhaustive-ctor.rs:25:39 | LL | let _ = S { field: (), field1: m::Priv1 {} }; | ------ ^^^^^ private struct | | | while setting this field | note: the struct `Priv1` is defined here --> $DIR/non-exhaustive-ctor.rs:14:4 | LL | struct Priv1 {} | ^^^^^^^^^^^^ help: the field `field1` you're trying to set has a default value, you can use `..` to use it | LL | let _ = S { field: (), .. }; | ~~ ```
217048f
to
29d26f2
Compare
@bors r=BoxyUwU |
…, r=BoxyUwU Detect struct construction with private field in field with default When trying to construct a struct that has a public field of a private type, suggest using `..` if that field has a default value. ``` error[E0603]: struct `Priv1` is private --> $DIR/non-exhaustive-ctor-2.rs:19:39 | LL | let _ = S { field: (), field1: m::Priv1 {} }; | ------ ^^^^^ private struct | | | while setting this field | note: the struct `Priv1` is defined here --> $DIR/non-exhaustive-ctor-2.rs:14:4 | LL | struct Priv1 {} | ^^^^^^^^^^^^ help: the type `Priv1` of field `field1` is private, but you can construct the default value defined for it in `S` using `..` in the struct initializer expression | LL | let _ = S { field: (), .. }; | ~~ ```
Rollup of 3 pull requests Successful merges: - #135846 (Detect struct construction with private field in field with default) - #144558 (Point at the `Fn()` or `FnMut()` bound that coerced a closure, which caused a move error) - #145149 (Make config method invoke inside parse use dwn_ctx) r? `@ghost` `@rustbot` modify labels: rollup
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing c8ca44c (parent) -> 21a19c2 (this PR) Test differencesShow 60 test diffsStage 1
Stage 2
Additionally, 58 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 21a19c297d4f5a03501d92ca251bd7a17073c08a --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (21a19c2): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.2%, secondary 0.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.5%, secondary 2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 464.929s -> 464.264s (-0.14%) |
When trying to construct a struct that has a public field of a private type, suggest using
..
if that field has a default value.