Skip to content

Commit

Permalink
Rollup merge of rust-lang#133545 - clubby789:symbol-intern-lit, r=jie…
Browse files Browse the repository at this point in the history
…youxu

Lint against Symbol::intern on a string literal

Disabled in tests where this doesn't make much sense
  • Loading branch information
matthiaskrgr authored Dec 3, 2024
2 parents e55607b + 71b698c commit 50f695c
Show file tree
Hide file tree
Showing 20 changed files with 94 additions and 37 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use rustc_ast::{NodeId, PatKind, attr, token};
use rustc_feature::{AttributeGate, BUILTIN_ATTRIBUTE_MAP, BuiltinAttribute, Features, GateIssue};
use rustc_session::Session;
use rustc_session::parse::{feature_err, feature_err_issue, feature_warn};
use rustc_span::Span;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::sym;
use rustc_span::{Span, Symbol};
use rustc_span::symbol::{Symbol, sym};
use rustc_target::spec::abi;
use thin_vec::ThinVec;

Expand Down Expand Up @@ -690,6 +690,7 @@ fn check_new_solver_banned_features(sess: &Session, features: &Features) {
.find(|feat| feat.gate_name == sym::generic_const_exprs)
.map(|feat| feat.attr_sp)
{
#[cfg_attr(not(bootstrap), allow(rustc::symbol_intern_string_literal))]
sess.dcx().emit_err(errors::IncompatibleFeatures {
spans: vec![gce_span],
f1: Symbol::intern("-Znext-solver=globally"),
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_borrowck/src/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use rustc_middle::ty::{
TyCtxt, TypeVisitableExt,
};
use rustc_middle::{bug, span_bug};
use rustc_span::ErrorGuaranteed;
use rustc_span::symbol::{kw, sym};
use rustc_span::{ErrorGuaranteed, Symbol};
use tracing::{debug, instrument};

use crate::BorrowckInferCtxt;
Expand Down Expand Up @@ -524,7 +524,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {

let reg_vid = self
.infcx
.next_nll_region_var(FR, || RegionCtxt::Free(Symbol::intern("c-variadic")))
.next_nll_region_var(FR, || RegionCtxt::Free(sym::c_dash_variadic))
.as_var();

let region = ty::Region::new_var(self.infcx.tcx, reg_vid);
Expand All @@ -540,10 +540,8 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
}
}

let fr_fn_body = self
.infcx
.next_nll_region_var(FR, || RegionCtxt::Free(Symbol::intern("fn_body")))
.as_var();
let fr_fn_body =
self.infcx.next_nll_region_var(FR, || RegionCtxt::Free(sym::fn_body)).as_var();

let num_universals = self.infcx.num_region_vars();

Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_codegen_cranelift/src/driver/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn create_jit_module(
jit_builder.symbol("__clif_jit_fn", clif_jit_fn as *const u8);
let mut jit_module = UnwindModule::new(JITModule::new(jit_builder), false);

let cx = crate::CodegenCx::new(tcx, jit_module.isa(), false, Symbol::intern("dummy_cgu_name"));
let cx = crate::CodegenCx::new(tcx, jit_module.isa(), false, sym::dummy_cgu_name);

crate::allocator::codegen(tcx, &mut jit_module);

Expand Down Expand Up @@ -276,12 +276,7 @@ fn jit_fn(instance_ptr: *const Instance<'static>, trampoline_ptr: *const u8) ->

jit_module.module.prepare_for_function_redefine(func_id).unwrap();

let mut cx = crate::CodegenCx::new(
tcx,
jit_module.isa(),
false,
Symbol::intern("dummy_cgu_name"),
);
let mut cx = crate::CodegenCx::new(tcx, jit_module.isa(), false, sym::dummy_cgu_name);
codegen_and_compile_fn(tcx, &mut cx, &mut Context::new(), jit_module, instance);

assert!(cx.global_asm.is_empty());
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,13 @@ impl CodegenBackend for CraneliftCodegenBackend {
// FIXME return the actually used target features. this is necessary for #[cfg(target_feature)]
if sess.target.arch == "x86_64" && sess.target.os != "none" {
// x86_64 mandates SSE2 support
vec![Symbol::intern("fxsr"), sym::sse, Symbol::intern("sse2")]
vec![sym::fsxr, sym::sse, sym::sse2]
} else if sess.target.arch == "aarch64" {
match &*sess.target.os {
"none" => vec![],
// On macOS the aes, sha2 and sha3 features are enabled by default and ring
// fails to compile on macOS when they are not present.
"macos" => vec![
sym::neon,
Symbol::intern("aes"),
Symbol::intern("sha2"),
Symbol::intern("sha3"),
],
"macos" => vec![sym::neon, sym::aes, sym::sha2, sym::sha3],
// AArch64 mandates Neon support
_ => vec![sym::neon],
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/collect/generics_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
if let Node::ConstBlock(_) = node {
own_params.push(ty::GenericParamDef {
index: next_index(),
name: Symbol::intern("<const_ty>"),
name: rustc_span::sym::const_ty_placeholder,
def_id: def_id.to_def_id(),
pure_wrt_drop: false,
kind: ty::GenericParamDefKind::Type { has_default: false, synthetic: false },
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let is_write = sugg_span.ctxt().outer_expn_data().macro_def_id.is_some_and(|def_id| {
tcx.is_diagnostic_item(sym::write_macro, def_id)
|| tcx.is_diagnostic_item(sym::writeln_macro, def_id)
}) && item_name.name == Symbol::intern("write_fmt");
}) && item_name.name == sym::write_fmt;
let mut err = if is_write && let SelfSource::MethodCall(rcvr_expr) = source {
self.suggest_missing_writer(rcvr_ty, rcvr_expr)
} else {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,9 @@ lint_suspicious_double_ref_clone =
lint_suspicious_double_ref_deref =
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
lint_symbol_intern_string_literal = using `Symbol::intern` on a string literal
.help = consider adding the symbol to `compiler/rustc_span/src/symbol.rs`
lint_trailing_semi_macro = trailing semicolon in macro used in expression position
.note1 = macro invocations at the end of a block are treated as expressions
.note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`
Expand Down
35 changes: 33 additions & 2 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ use tracing::debug;

use crate::lints::{
BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword,
NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked, SpanUseEqCtxtDiag, TyQualified,
TykindDiag, TykindKind, TypeIrInherentUsage, UntranslatableDiag,
NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked, SpanUseEqCtxtDiag,
SymbolInternStringLiteralDiag, TyQualified, TykindDiag, TykindKind, TypeIrInherentUsage,
UntranslatableDiag,
};
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};

Expand Down Expand Up @@ -650,3 +651,33 @@ fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
_ => false,
}
}

declare_tool_lint! {
/// The `symbol_intern_string_literal` detects `Symbol::intern` being called on a string literal
pub rustc::SYMBOL_INTERN_STRING_LITERAL,
// rustc_driver crates out of the compiler can't/shouldn't add preinterned symbols;
// bootstrap will deny this manually
Allow,
"Forbid uses of string literals in `Symbol::intern`, suggesting preinterning instead",
report_in_external_macro: true
}

declare_lint_pass!(SymbolInternStringLiteral => [SYMBOL_INTERN_STRING_LITERAL]);

impl<'tcx> LateLintPass<'tcx> for SymbolInternStringLiteral {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
if let ExprKind::Call(path, [arg]) = expr.kind
&& let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::SymbolIntern, def_id)
&& let ExprKind::Lit(kind) = arg.kind
&& let rustc_ast::LitKind::Str(_, _) = kind.node
{
cx.emit_span_lint(
SYMBOL_INTERN_STRING_LITERAL,
kind.span,
SymbolInternStringLiteralDiag,
);
}
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,8 @@ fn register_internals(store: &mut LintStore) {
store.register_late_mod_pass(|_| Box::new(PassByValue));
store.register_lints(&SpanUseEqCtxt::lint_vec());
store.register_late_mod_pass(|_| Box::new(SpanUseEqCtxt));
store.register_lints(&SymbolInternStringLiteral::lint_vec());
store.register_late_mod_pass(|_| Box::new(SymbolInternStringLiteral));
// FIXME(davidtwco): deliberately do not include `UNTRANSLATABLE_DIAGNOSTIC` and
// `DIAGNOSTIC_OUTSIDE_OF_IMPL` here because `-Wrustc::internal` is provided to every crate and
// these lints will trigger all of the time - change this once migration to diagnostic structs
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,11 @@ pub(crate) struct QueryUntracked {
#[diag(lint_span_use_eq_ctxt)]
pub(crate) struct SpanUseEqCtxtDiag;

#[derive(LintDiagnostic)]
#[diag(lint_symbol_intern_string_literal)]
#[help]
pub(crate) struct SymbolInternStringLiteralDiag;

#[derive(LintDiagnostic)]
#[diag(lint_tykind_kind)]
pub(crate) struct TykindKind {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ impl UnusedImportBraces {
}
rename.unwrap_or(orig_ident).name
}
ast::UseTreeKind::Glob => Symbol::intern("*"),
ast::UseTreeKind::Glob => sym::asterisk,
ast::UseTreeKind::Nested { .. } => return,
};

Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,8 +861,10 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
// First up we check for global allocators. Look at the crate graph here
// and see what's a global allocator, including if we ourselves are a
// global allocator.
let mut global_allocator =
self.cstore.has_global_allocator.then(|| Symbol::intern("this crate"));
#[cfg_attr(not(bootstrap), allow(rustc::symbol_intern_string_literal))]
let this_crate = Symbol::intern("this crate");

let mut global_allocator = self.cstore.has_global_allocator.then_some(this_crate);
for (_, data) in self.cstore.iter_crate_data() {
if data.has_global_allocator() {
match global_allocator {
Expand All @@ -876,8 +878,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
}
}
}
let mut alloc_error_handler =
self.cstore.has_alloc_error_handler.then(|| Symbol::intern("this crate"));
let mut alloc_error_handler = self.cstore.has_alloc_error_handler.then_some(this_crate);
for (_, data) in self.cstore.iter_crate_data() {
if data.has_alloc_error_handler() {
match alloc_error_handler {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,7 @@ impl MetadataBlob {

let def_kind = root.tables.def_kind.get(blob, item).unwrap();
let def_key = root.tables.def_keys.get(blob, item).unwrap().decode(blob);
#[cfg_attr(not(bootstrap), allow(rustc::symbol_intern_string_literal))]
let def_name = if item == CRATE_DEF_INDEX {
rustc_span::symbol::kw::Crate
} else {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_ast::{
};
use rustc_errors::{Applicability, PResult};
use rustc_span::symbol::{Ident, kw, sym};
use rustc_span::{ErrorGuaranteed, Span, Symbol};
use rustc_span::{ErrorGuaranteed, Span};
use thin_vec::{ThinVec, thin_vec};

use super::{Parser, PathStyle, SeqSep, TokenType, Trailing};
Expand Down Expand Up @@ -1136,7 +1136,7 @@ impl<'a> Parser<'a> {
Some(ast::Path {
span: fn_token_span.to(self.prev_token.span),
segments: thin_vec![ast::PathSegment {
ident: Ident::new(Symbol::intern("Fn"), fn_token_span),
ident: Ident::new(sym::Fn, fn_token_span),
id: DUMMY_NODE_ID,
args: Some(P(ast::GenericArgs::Parenthesized(ast::ParenthesizedArgs {
span: args_lo.to(self.prev_token.span),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3113,6 +3113,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
}
}

#[cfg_attr(not(bootstrap), allow(rustc::symbol_intern_string_literal))]
let existing_name = match &in_scope_lifetimes[..] {
[] => Symbol::intern("'a"),
[(existing, _)] => existing.name,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2257,7 +2257,7 @@ fn module_to_string(module: Module<'_>) -> Option<String> {
collect_mod(names, parent);
}
} else {
names.push(Symbol::intern("<opaque>"));
names.push(sym::opaque_module_name_placeholder);
collect_mod(names, module.parent.unwrap());
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub fn find_crate_name(sess: &Session, attrs: &[ast::Attribute]) -> Symbol {
}
}

Symbol::intern("rust_out")
sym::rust_out
}

pub fn validate_crate_name(sess: &Session, s: Symbol, sp: Option<Span>) {
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ symbols! {
Return,
Right,
Rust,
RustaceansAreAwesome,
RustcDecodable,
RustcEncodable,
RwLock,
Expand All @@ -316,6 +317,7 @@ symbols! {
StructuralPartialEq,
SubdiagMessage,
Subdiagnostic,
SymbolIntern,
Sync,
SyncUnsafeCell,
T,
Expand Down Expand Up @@ -377,6 +379,7 @@ symbols! {
adt_const_params,
advanced_slice_patterns,
adx_target_feature,
aes,
aggregate_raw_ptr,
alias,
align,
Expand Down Expand Up @@ -440,6 +443,7 @@ symbols! {
associated_types,
assume,
assume_init,
asterisk: "*",
async_await,
async_call,
async_call_mut,
Expand Down Expand Up @@ -520,6 +524,7 @@ symbols! {
btreeset_iter,
builtin_syntax,
c,
c_dash_variadic,
c_str,
c_str_literals,
c_unwind,
Expand Down Expand Up @@ -650,6 +655,7 @@ symbols! {
const_trait_bound_opt_out,
const_trait_impl,
const_try,
const_ty_placeholder: "<const_ty>",
constant,
constructor,
convert_identity,
Expand Down Expand Up @@ -779,6 +785,7 @@ symbols! {
drop_types_in_const,
dropck_eyepatch,
dropck_parametricity,
dummy_cgu_name,
dylib,
dyn_compatible_for_dispatch,
dyn_metadata,
Expand Down Expand Up @@ -922,6 +929,7 @@ symbols! {
fmuladdf32,
fmuladdf64,
fn_align,
fn_body,
fn_delegation,
fn_must_use,
fn_mut,
Expand Down Expand Up @@ -962,6 +970,7 @@ symbols! {
fs_create_dir,
fsub_algebraic,
fsub_fast,
fsxr,
full,
fundamental,
fused_iterator,
Expand Down Expand Up @@ -1385,6 +1394,7 @@ symbols! {
on,
on_unimplemented,
opaque,
opaque_module_name_placeholder: "<opaque>",
open_options_new,
ops,
opt_out_copy,
Expand Down Expand Up @@ -1654,6 +1664,7 @@ symbols! {
rust_eh_catch_typeinfo,
rust_eh_personality,
rust_logo,
rust_out,
rustc,
rustc_abi,
rustc_allocator,
Expand Down Expand Up @@ -1776,6 +1787,8 @@ symbols! {
self_in_typedefs,
self_struct_ctor,
semitransparent,
sha2,
sha3,
sha512_sm_x86,
shadow_call_stack,
shallow,
Expand Down Expand Up @@ -1889,6 +1902,7 @@ symbols! {
sreg,
sreg_low16,
sse,
sse2,
sse4a_target_feature,
stable,
staged_api,
Expand Down Expand Up @@ -2176,6 +2190,7 @@ symbols! {
wrapping_sub,
wreg,
write_bytes,
write_fmt,
write_macro,
write_str,
write_via_move,
Expand Down Expand Up @@ -2405,6 +2420,7 @@ impl Symbol {
}

/// Maps a string to its interned representation.
#[rustc_diagnostic_item = "SymbolIntern"]
pub fn intern(string: &str) -> Self {
with_session_globals(|session_globals| session_globals.symbol_interner.intern(string))
}
Expand Down
Loading

0 comments on commit 50f695c

Please sign in to comment.