Lint against Symbol::intern on a string literal #133545

clubby789 · 2024-11-27T17:55:37Z

Disabled in tests where this doesn't make much sense

rustbot · 2024-11-27T17:55:46Z

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-11-27T18:24:08Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

jieyouxu · 2024-11-27T19:05:28Z

r? jieyouxu

compiler/rustc_ast_passes/src/feature_gate.rs

jieyouxu

Thanks, I concur with previous remarks that this is should not be universally applied.

compiler/rustc_hir_analysis/src/collect/generics_of.rs

compiler/rustc_metadata/src/creader.rs

compiler/rustc_metadata/src/rmeta/decoder.rs

compiler/rustc_resolve/src/late/diagnostics.rs

compiler/rustc_resolve/src/lib.rs

compiler/rustc_trait_selection/src/traits/dyn_compatibility.rs

compiler/rustc_borrowck/src/universal_regions.rs

clubby789 · 2024-11-28T02:58:05Z

Reverted the cases where this is less than helpful, but I've left the others to match the existing symbol naming rules.

Then again, it looks like the rule isn't very consistently applied (there's a few symbols with dashes which don't contain _dash_), so if you feel strongly about the mentioned cases we can adjust them

jieyouxu · 2024-11-28T03:34:47Z

In my opinion, if the sym names derived from strictly following the sym naming rules turns out to be less readable, then:

(1) Either we don't make it a sym,
(2) Or we don't strictly follow the naming rules.

For stuff like <xxx> becoming lt_xxx_rt, I think that's significantly worse readability wise if we convert them into sym and strictly follow said sym naming rules. The c-variadic case I don't think is as problematic, but it might be less obvious than actually c-variadic as-is.

compiler/rustc_lint/src/internal.rs

clubby789 · 2024-11-28T15:47:52Z

Switched to only passing the -D from bootstrap if we're not running a test command

jieyouxu

Thanks!

jieyouxu · 2024-12-02T23:18:25Z

@bors r+ rollup

bors · 2024-12-02T23:18:28Z

📌 Commit 71b698c has been approved by jieyouxu

It is now in the queue for this repository.

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#133089 (Stabilize noop_waker) - rust-lang#133522 (Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly) - rust-lang#133545 (Lint against Symbol::intern on a string literal) - rust-lang#133558 (Structurally resolve in `probe_adt`) - rust-lang#133753 (Reduce false positives on some common cases from if-let-rescope lint) - rust-lang#133762 (stabilize const_{size,align}_of_val) - rust-lang#133777 (document -Zrandomize-layout in the unstable book) - rust-lang#133779 (Use correct `hir_id` for array const arg infers) r? `@ghost` `@rustbot` modify labels: rollup

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#133522 (Don't suggest restricting bound with unstable traits on stable and mention it's unstable on nightly) - rust-lang#133545 (Lint against Symbol::intern on a string literal) - rust-lang#133558 (Structurally resolve in `probe_adt`) - rust-lang#133753 (Reduce false positives on some common cases from if-let-rescope lint) - rust-lang#133762 (stabilize const_{size,align}_of_val) - rust-lang#133777 (document -Zrandomize-layout in the unstable book) - rust-lang#133779 (Use correct `hir_id` for array const arg infers) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#132612 (Gate async fn trait bound modifier on `async_trait_bounds`) - rust-lang#133545 (Lint against Symbol::intern on a string literal) - rust-lang#133558 (Structurally resolve in `probe_adt`) - rust-lang#133696 (stabilize const_collections_with_hasher and build_hasher_default_const_new) - rust-lang#133753 (Reduce false positives on some common cases from if-let-rescope lint) - rust-lang#133762 (stabilize const_{size,align}_of_val) - rust-lang#133777 (document -Zrandomize-layout in the unstable book) - rust-lang#133779 (Use correct `hir_id` for array const arg infers) - rust-lang#133796 (Update the definition of `borrowing_sub`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#133545 - clubby789:symbol-intern-lit, r=jieyouxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

…ing-literal, r=jieyouxu allow `symbol_intern_string_literal` lint in test modules Since rust-lang#133545, `x check compiler --stage 1` no longer works because compiler test modules trigger `symbol_intern_string_literal` lint errors. Bootstrap shouldn't control when to ignore or enable this lint in the compiler tree (using `Kind != Test` was ineffective for obvious reasons). Also, conditionally adding this rustflag invalidates the build cache between `x test` and other commands. This PR removes the `Kind` check from bootstrap and handles it directly in the compiler tree in a more natural way.

Rollup merge of rust-lang#134173 - onur-ozkan:allow-symbol-intern-string-literal, r=jieyouxu allow `symbol_intern_string_literal` lint in test modules Since rust-lang#133545, `x check compiler --stage 1` no longer works because compiler test modules trigger `symbol_intern_string_literal` lint errors. Bootstrap shouldn't control when to ignore or enable this lint in the compiler tree (using `Kind != Test` was ineffective for obvious reasons). Also, conditionally adding this rustflag invalidates the build cache between `x test` and other commands. This PR removes the `Kind` check from bootstrap and handles it directly in the compiler tree in a more natural way.

rustbot assigned Nadrieril Nov 27, 2024

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 Nov 27, 2024

This comment has been minimized.

Sign in to view

clubby789 force-pushed the symbol-intern-lit branch from f8edb84 to ebe0e42 Compare November 27, 2024 18:24

This comment has been minimized.

Sign in to view

clubby789 force-pushed the symbol-intern-lit branch from ebe0e42 to 4d6e8f8 Compare November 27, 2024 19:04

rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Nov 27, 2024

rustbot assigned jieyouxu and unassigned Nadrieril Nov 27, 2024

clubby789 commented Nov 27, 2024

View reviewed changes

compiler/rustc_ast_passes/src/feature_gate.rs Outdated Show resolved Hide resolved

clubby789 force-pushed the symbol-intern-lit branch from 4d6e8f8 to 1619597 Compare November 28, 2024 01:02

jieyouxu requested changes Nov 28, 2024

View reviewed changes

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 Nov 28, 2024

clubby789 force-pushed the symbol-intern-lit branch from 1619597 to dd442e4 Compare November 28, 2024 02:52

Urgau reviewed Nov 28, 2024

View reviewed changes

compiler/rustc_lint/src/internal.rs Outdated Show resolved Hide resolved

clubby789 added 2 commits November 28, 2024 15:45

Implement lint against Symbol::intern on a string literal

72d2db7

Replace Symbol::intern calls with preinterned symbols

71b698c

clubby789 force-pushed the symbol-intern-lit branch from dd442e4 to 71b698c Compare November 28, 2024 15:45

clubby789 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 Dec 2, 2024

jieyouxu approved these changes Dec 2, 2024

View reviewed changes

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 Dec 2, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 3, 2024

Rollup merge of rust-lang#133545 - clubby789:symbol-intern-lit, r=jie…

fa58c6b

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

matthiaskrgr mentioned this pull request Dec 3, 2024

Rollup of 8 pull requests #133783

Closed

jhpratt added a commit to jhpratt/rust that referenced this pull request Dec 3, 2024

Rollup merge of rust-lang#133545 - clubby789:symbol-intern-lit, r=jie…

90b30d4

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

jhpratt mentioned this pull request Dec 3, 2024

Rollup of 10 pull requests #133786

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 3, 2024

Rollup merge of rust-lang#133545 - clubby789:symbol-intern-lit, r=jie…

50f695c

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

matthiaskrgr mentioned this pull request Dec 3, 2024

Rollup of 7 pull requests #133787

Closed

matthiaskrgr mentioned this pull request Dec 3, 2024

Rollup of 9 pull requests #133803

Merged

bors merged commit 453a1a8 into rust-lang:master Dec 3, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 3, 2024

bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Dec 6, 2024

Rollup merge of rust-lang#133545 - clubby789:symbol-intern-lit, r=jie…

7174bcc

…youxu Lint against Symbol::intern on a string literal Disabled in tests where this doesn't make much sense

onur-ozkan mentioned this pull request Dec 11, 2024

allow symbol_intern_string_literal lint in test modules #134173

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lint against Symbol::intern on a string literal #133545

Lint against Symbol::intern on a string literal #133545

clubby789 commented Nov 27, 2024

rustbot commented Nov 27, 2024

This comment has been minimized.

rustbot commented Nov 27, 2024

This comment has been minimized.

jieyouxu commented Nov 27, 2024

jieyouxu left a comment

clubby789 commented Nov 28, 2024

jieyouxu commented Nov 28, 2024

clubby789 commented Nov 28, 2024

jieyouxu left a comment

jieyouxu commented Dec 2, 2024

bors commented Dec 2, 2024

Lint against Symbol::intern on a string literal #133545

Lint against Symbol::intern on a string literal #133545

Conversation

clubby789 commented Nov 27, 2024

rustbot commented Nov 27, 2024

This comment has been minimized.

rustbot commented Nov 27, 2024

This comment has been minimized.

jieyouxu commented Nov 27, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

clubby789 commented Nov 28, 2024

jieyouxu commented Nov 28, 2024

clubby789 commented Nov 28, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Dec 2, 2024

bors commented Dec 2, 2024