Skip to content

Conversation

petrochenkov
Copy link
Contributor

And a couple of other naming and comment tweaks.

Related to #48054

For enum Level I initially used naming enum EffectiveVisibilityLevel, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just Level, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like use rustc_middle::privacy::Level as EffVisLevel or something.

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 24, 2022
@rust-highfive
Copy link
Contributor

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 24, 2022
@petrochenkov
Copy link
Contributor Author

cc @Bryanskiy

@bors

This comment was marked as resolved.

@camsteffen
Copy link
Contributor

How about Visibilities and VisibilityLevel?

@petrochenkov
Copy link
Contributor Author

@camsteffen

How about Visibilities and VisibilityLevel?

To avoid confusing effective visibilities with "primitive"/"basic" ty::Visibilityes.
A map that contains EffectiveVisibilityes -> EffectiveVisibilities.

As for Level, I explained it in the top comment, it's mainly for brevity because it's commonly used.

@camsteffen
Copy link
Contributor

I see. Maybe Visibility could be named VisibilityDecl?

@petrochenkov
Copy link
Contributor Author

Ping @jackh726, it's been almost a month.

@jackh726
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 23, 2022

📌 Commit 8e84ee5 has been approved by jackh726

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2022
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Oct 25, 2022
privacy: Rename "accessibility levels" to "effective visibilities"

And a couple of other naming and comment tweaks.

Related to rust-lang#48054

For `enum Level` I initially used naming `enum EffectiveVisibilityLevel`, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just `Level`, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like `use rustc_middle::privacy::Level as EffVisLevel` or something.
@bors

This comment was marked as resolved.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2022
@petrochenkov
Copy link
Contributor Author

@bors r=jackh726

@bors
Copy link
Collaborator

bors commented Oct 26, 2022

📌 Commit fba05d139d8641e4b955dc2659820229e21f2f09 has been approved by jackh726

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2022
@bors

This comment was marked as resolved.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2022
@petrochenkov
Copy link
Contributor Author

@bors r=jackh726

@bors
Copy link
Collaborator

bors commented Oct 26, 2022

📌 Commit 34eb73c has been approved by jackh726

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2022
@Dylan-DPC
Copy link
Member

@bors rollup=iffy

@petrochenkov
Copy link
Contributor Author

Not sure why this PR was marked as iffy, it can be rolled up.
The only danger here is potential merge conflicts.

@bors
Copy link
Collaborator

bors commented Oct 29, 2022

⌛ Testing commit 34eb73c with merge 33b55ac...

@Dylan-DPC
Copy link
Member

yeh it was marked as iffy because of the chance of merge conflicts :P

@bors
Copy link
Collaborator

bors commented Oct 29, 2022

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 33b55ac to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (33b55ac): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

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
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.3% [-1.4%, -1.2%] 2
Improvements ✅
(secondary)
-2.0% [-4.0%, -0.4%] 12
All ❌✅ (primary) -1.3% [-1.4%, -1.2%] 2

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
Regressions ❌
(secondary)
3.3% [3.3%, 3.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% [-3.4%, -2.8%] 3
All ❌✅ (primary) - - 0

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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.5% [-2.7%, -2.2%] 6
All ❌✅ (primary) - - 0

@lqd
Copy link
Member

lqd commented Oct 29, 2022

Note for weekly triage: the wins could be bimodal noise on these benchmarks. They're the opposite of the previous PR on these 14 benchmarks, in the second spike below.

image

@petrochenkov
Copy link
Contributor Author

This PR only contains renamings, so it's certainly noise.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 21, 2022
privacy: Rename "accessibility levels" to "effective visibilities"

And a couple of other naming and comment tweaks.

Related to rust-lang#48054

For `enum Level` I initially used naming `enum EffectiveVisibilityLevel`, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just `Level`, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like `use rustc_middle::privacy::Level as EffVisLevel` or something.
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
privacy: Rename "accessibility levels" to "effective visibilities"

And a couple of other naming and comment tweaks.

Related to rust-lang#48054

For `enum Level` I initially used naming `enum EffectiveVisibilityLevel`, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just `Level`, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like `use rustc_middle::privacy::Level as EffVisLevel` or something.
@petrochenkov petrochenkov deleted the effvis branch February 22, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc 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