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.

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

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

Merged
merged 2 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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))
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
.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 @@ -772,6 +772,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 @@ -657,3 +658,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"));
clubby789 marked this conversation as resolved.
Show resolved Hide resolved
#[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 @@ -1139,7 +1139,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 @@ -2270,7 +2270,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 @@ -297,6 +297,7 @@ symbols! {
Return,
Right,
Rust,
RustaceansAreAwesome,
RustcDecodable,
RustcEncodable,
RwLock,
Expand All @@ -315,6 +316,7 @@ symbols! {
StructuralPartialEq,
SubdiagMessage,
Subdiagnostic,
SymbolIntern,
Sync,
SyncUnsafeCell,
T,
Expand Down Expand Up @@ -376,6 +378,7 @@ symbols! {
adt_const_params,
advanced_slice_patterns,
adx_target_feature,
aes,
aggregate_raw_ptr,
alias,
align,
Expand Down Expand Up @@ -438,6 +441,7 @@ symbols! {
associated_types,
assume,
assume_init,
asterisk: "*",
async_await,
async_call,
async_call_mut,
Expand Down Expand Up @@ -518,6 +522,7 @@ symbols! {
btreeset_iter,
builtin_syntax,
c,
c_dash_variadic,
c_str,
c_str_literals,
c_unwind,
Expand Down Expand Up @@ -648,6 +653,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 @@ -777,6 +783,7 @@ symbols! {
drop_types_in_const,
dropck_eyepatch,
dropck_parametricity,
dummy_cgu_name,
dylib,
dyn_compatible_for_dispatch,
dyn_metadata,
Expand Down Expand Up @@ -920,6 +927,7 @@ symbols! {
fmuladdf32,
fmuladdf64,
fn_align,
fn_body,
fn_delegation,
fn_must_use,
fn_mut,
Expand Down Expand Up @@ -960,6 +968,7 @@ symbols! {
fs_create_dir,
fsub_algebraic,
fsub_fast,
fsxr,
full,
fundamental,
fused_iterator,
Expand Down Expand Up @@ -1383,6 +1392,7 @@ symbols! {
on,
on_unimplemented,
opaque,
opaque_module_name_placeholder: "<opaque>",
open_options_new,
ops,
opt_out_copy,
Expand Down Expand Up @@ -1651,6 +1661,7 @@ symbols! {
rust_eh_catch_typeinfo,
rust_eh_personality,
rust_logo,
rust_out,
rustc,
rustc_abi,
rustc_allocator,
Expand Down Expand Up @@ -1773,6 +1784,8 @@ symbols! {
self_in_typedefs,
self_struct_ctor,
semitransparent,
sha2,
sha3,
sha512_sm_x86,
shadow_call_stack,
shallow,
Expand Down Expand Up @@ -1886,6 +1899,7 @@ symbols! {
sreg,
sreg_low16,
sse,
sse2,
sse4a_target_feature,
stable,
staged_api,
Expand Down Expand Up @@ -2172,6 +2186,7 @@ symbols! {
wrapping_sub,
wreg,
write_bytes,
write_fmt,
write_macro,
write_str,
write_via_move,
Expand Down Expand Up @@ -2401,6 +2416,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
Loading