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.

Sign up for GitHub

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

Jump to bottom

Populate effective visibilities in rustc_privacy (take 2) #111425

Merged
merged 1 commit into from
May 14, 2023
Merged

Populate effective visibilities in rustc_privacy (take 2) #111425

merged 1 commit into from
May 14, 2023

Conversation

Bryanskiy
Copy link
Contributor

@Bryanskiy Bryanskiy commented May 10, 2023 •
edited by petrochenkov
Loading

Same as #110907 + regressions fixes.
Fixes #111359.

r? @petrochenkov

@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 10, 2023
@Bryanskiy
Copy link
Contributor Author

@rustbot author

@rustbot 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 May 10, 2023
@rustbot
Copy link
Collaborator

rustbot commented May 10, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@petrochenkov petrochenkov changed the title Populate effective visibilities in rustc_privacy Populate effective visibilities in rustc_privacy (take 2) May 10, 2023
@Bryanskiy
Copy link
Contributor Author

@rustbot ready

@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 May 10, 2023
@petrochenkov petrochenkov mentioned this pull request May 10, 2023
Closed
@petrochenkov petrochenkov 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 May 10, 2023
@Bryanskiy
Copy link
Contributor Author

@rustbot ready

@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 May 11, 2023
@petrochenkov
Copy link
Contributor

@bors try

@bors
Copy link
Contributor

bors commented May 11, 2023

⌛ Trying commit 670f5b1 with merge 99a5dd890067998fd41367b049bb23a68a999944...

@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2023
@bors
Copy link
Contributor

bors commented May 11, 2023

☀️ Try build successful - checks-actions
Build commit: 99a5dd890067998fd41367b049bb23a68a999944 (99a5dd890067998fd41367b049bb23a68a999944)

@petrochenkov
Copy link
Contributor

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-111425 created and queued.
🤖 Automatically detected try build 99a5dd890067998fd41367b049bb23a68a999944
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🚧 Experiment pr-111425 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🚨 Report generation of pr-111425 failed: Failed to upload to "try#99a5dd890067998fd41367b049bb23a68a999944/gh/sanket143.windows143/log.txt": ServiceError(ServiceError { source: PutObjectError { kind: Unhandled(Unhandled { source: Error { code: Some("InternalError"), message: Some("We encountered an internal error. Please try again."), request_id: Some("PHXA3SMEP1ENDQXW"), extras: {"s3_extended_request_id": "+PaVDrlSwUxtH7xki7yrPhck6VrcqiBI3qW2ABkoP464b8IoMJGgWPRztzJvkGiRdpfNj6Oe6Gg="} } }), meta: Error { code: Some("InternalError"), message: Some("We encountered an internal error. Please try again."), request_id: Some("PHXA3SMEP1ENDQXW"), extras: {"s3_extended_request_id": "+PaVDrlSwUxtH7xki7yrPhck6VrcqiBI3qW2ABkoP464b8IoMJGgWPRztzJvkGiRdpfNj6Oe6Gg="} } }, raw: Response { inner: Response { status: 500, version: HTTP/1.1, headers: {"x-amz-request-id": "PHXA3SMEP1ENDQXW", "x-amz-id-2": "+PaVDrlSwUxtH7xki7yrPhck6VrcqiBI3qW2ABkoP464b8IoMJGgWPRztzJvkGiRdpfNj6Oe6Gg=", "content-type": "application/xml", "transfer-encoding": "chunked", "date": "Fri, 12 May 2023 12:20:04 GMT", "server": "AmazonS3", "connection": "close"}, body: SdkBody { inner: Once(Some(b"\nInternalErrorWe encountered an internal error. Please try again.PHXA3SMEP1ENDQXW+PaVDrlSwUxtH7xki7yrPhck6VrcqiBI3qW2ABkoP464b8IoMJGgWPRztzJvkGiRdpfNj6Oe6Gg=")), retryable: true } }, properties: SharedPropertyBag(Mutex { data: PropertyBag, poisoned: false, .. }) } })
🛠️ If the error is fixed use the retry-report command.

🆘 Can someone from the infra team check in on this? @rust-lang/infra
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@petrochenkov
Copy link
Contributor

Not sure why the report wasn't generated.
Logs for regressed crates can be found in https://crater-reports.s3.amazonaws.com/pr-111425/logs-archives/regressed.tar.gz, all the regressions are spurious.
@bors r+

@bors
Copy link
Contributor

bors commented May 12, 2023

📌 Commit 670f5b1 has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 12, 2023
@bors
Copy link
Contributor

bors commented May 14, 2023

⌛ Testing commit 670f5b1 with merge ad6ab11...

@bors
Copy link
Contributor

bors commented May 14, 2023

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing ad6ab11 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 14, 2023
@bors bors merged commit ad6ab11 into rust-lang:master May 14, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 14, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ad6ab11): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.7% [0.4%, 1.5%] 25
Regressions ❌
(secondary)		 0.6% [0.3%, 1.0%] 10
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.7% [0.4%, 1.5%] 25

Max RSS (memory usage)

Results

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.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)		 2.4% [2.4%, 2.4%] 1
Improvements ✅
(primary)		 -1.9% [-1.9%, -1.9%] 1
Improvements ✅
(secondary)		 -2.2% [-2.8%, -1.6%] 2
All ❌✅ (primary) -0.7% [-1.9%, 0.4%] 2

Cycles

Results

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)		 1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -6.9% [-8.7%, -1.9%] 4
All ❌✅ (primary) 1.1% [1.1%, 1.1%] 1

Binary size

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

Bootstrap: 659.769s -> 661.349s (0.24%)

@rustbot rustbot added the perf-regression Performance regression. label May 14, 2023
@petrochenkov
Copy link
Contributor

Analysis of the perf regressions - #110907 (comment).

#111260 should compensate for most of them.

@petrochenkov
Copy link
Contributor

@craterbot abort

Crate failed with "Report failed", but the PR was never removed from the crater dashboard.

@petrochenkov
Copy link
Contributor

@craterbot abort name=pr-111425

@craterbot
Copy link
Collaborator

🗑️ Experiment pr-111425 deleted!

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 31, 2023
petrochenkov added a commit to petrochenkov/rust that referenced this pull request Jun 15, 2023
@petrochenkov
privacy: Do not mark items reachable farther than their nominal visib…
95a24c6 
…ility

This commit reverts a change made in rust-lang#111425.
It was believed that this change was necessary for implementing type privacy lints, but rust-lang#111801 showed that it was not necessary.
Quite opposite, the revert fixes some issues.
@lqd lqd mentioned this pull request Aug 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. 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
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

Regression: missing_debug_implementations linting on private items when used as parameter in trait impl
6 participants
@Bryanskiy @rustbot @petrochenkov @bors @craterbot @rust-timer