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

add test to reproduce #137687 and fix it by converting #[crate_name] to a new-style attribute parser #137729

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
26 changes: 26 additions & 0 deletions compiler/rustc_attr_data_structures/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ pub enum AttributeKind {
span: Span,
},
ConstStabilityIndirect,
CrateName {
name: Symbol,
name_span: Span,
Comment on lines +179 to +180
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could theoretically use Spanned but it doesn't necessarily lead to more readable code (and I don't remember if it's compatible with translatable diagnostics).

Suggested change
name: Symbol,
name_span: Span,
name: Symbol,
name_span: Span,

style: AttrStyle,
},
Deprecation {
deprecation: Deprecation,
span: Span,
Expand All @@ -195,3 +200,24 @@ pub enum AttributeKind {
},
// tidy-alphabetical-end
}

impl AttributeKind {
pub fn crate_level(&self) -> Option<(AttrStyle, Span)> {
match self {
AttributeKind::AllowConstFnUnstable(..)
| AttributeKind::AllowInternalUnstable(..)
| AttributeKind::BodyStability { .. }
| AttributeKind::Confusables { .. }
| AttributeKind::ConstStability { .. }
| AttributeKind::ConstStabilityIndirect
| AttributeKind::Deprecation { .. }
| AttributeKind::Diagnostic(..)
| AttributeKind::DocComment { .. }
| AttributeKind::MacroTransparency(..)
| AttributeKind::Repr(..)
| AttributeKind::Stability { .. } => None,

AttributeKind::CrateName { style, name_span, .. } => Some((*style, *name_span)),
}
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_attr_parsing/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ attr_parsing_multiple_item =
attr_parsing_multiple_stability_levels =
multiple stability levels

attr_parsing_name_value = malformed `{$name}` attribute: expected to be of the form `#[{$name} = ...]`

attr_parsing_non_ident_feature =
'feature' is not an identifier

Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_attr_parsing/src/attributes/crate_name.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use rustc_attr_data_structures::AttributeKind;
use rustc_span::{Span, Symbol, sym};

use super::SingleAttributeParser;
use crate::context::AcceptContext;
use crate::parser::ArgParser;
use crate::session_diagnostics::{ExpectedNameValue, IncorrectMetaItem, UnusedMultiple};

pub(crate) struct CratenameParser;

impl SingleAttributeParser for CratenameParser {
const PATH: &'static [Symbol] = &[sym::crate_name];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: You generally no longer need to explicitly state 'static in the ty of assoc consts.


fn on_duplicate(cx: &AcceptContext<'_>, first_span: Span) {
// FIXME(jdonszelmann): better duplicate reporting (WIP)
cx.emit_err(UnusedMultiple {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicates are warnings on master, right? So this is a breaking change?

this: cx.attr_span,
other: first_span,
name: sym::crate_name,
});
}

fn convert(cx: &AcceptContext<'_>, args: &ArgParser<'_>) -> Option<AttributeKind> {
if let ArgParser::NameValue(n) = args {
if let Some(name) = n.value_as_str() {
Some(AttributeKind::CrateName {
name,
name_span: n.value_span,
style: cx.attr_style,
})
} else {
cx.emit_err(IncorrectMetaItem { span: cx.attr_span, suggestion: None });

None
}
} else {
cx.emit_err(ExpectedNameValue { span: cx.attr_span, name: sym::crate_name });

None
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_attr_parsing/src/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::parser::ArgParser;
pub(crate) mod allow_unstable;
pub(crate) mod cfg;
pub(crate) mod confusables;
pub(crate) mod crate_name;
pub(crate) mod deprecation;
pub(crate) mod repr;
pub(crate) mod stability;
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_attr_parsing/src/attributes/util.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_ast::attr::{AttributeExt, first_attr_value_str_by_name};
use rustc_ast::attr::AttributeExt;
use rustc_attr_data_structures::RustcVersion;
use rustc_feature::is_builtin_attr_name;
use rustc_span::{Symbol, sym};
use rustc_span::Symbol;

/// Parse a rustc version number written inside string literal in an attribute,
/// like appears in `since = "1.0.0"`. Suffixes like "-dev" and "-nightly" are
Expand All @@ -22,7 +22,3 @@ pub fn parse_version(s: Symbol) -> Option<RustcVersion> {
pub fn is_builtin_attr(attr: &impl AttributeExt) -> bool {
attr.is_doc_comment() || attr.ident().is_some_and(|ident| is_builtin_attr_name(ident.name))
}

pub fn find_crate_name(attrs: &[impl AttributeExt]) -> Option<Symbol> {
first_attr_value_str_by_name(attrs, sym::crate_name)
}
6 changes: 5 additions & 1 deletion compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::BTreeMap;
use std::ops::Deref;
use std::sync::LazyLock;

use rustc_ast::{self as ast, DelimArgs};
use rustc_ast::{self as ast, AttrStyle, DelimArgs};
use rustc_attr_data_structures::AttributeKind;
use rustc_errors::{DiagCtxtHandle, Diagnostic};
use rustc_feature::Features;
Expand All @@ -14,6 +14,7 @@ use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Symbol, sym};

use crate::attributes::allow_unstable::{AllowConstFnUnstableParser, AllowInternalUnstableParser};
use crate::attributes::confusables::ConfusablesParser;
use crate::attributes::crate_name::CratenameParser;
use crate::attributes::deprecation::DeprecationParser;
use crate::attributes::repr::ReprParser;
use crate::attributes::stability::{
Expand Down Expand Up @@ -76,6 +77,7 @@ attribute_groups!(

// tidy-alphabetical-start
Single<ConstStabilityIndirectParser>,
Single<CratenameParser>,
Single<DeprecationParser>,
Single<TransparencyParser>,
// tidy-alphabetical-end
Expand All @@ -89,6 +91,7 @@ pub(crate) struct AcceptContext<'a> {
pub(crate) group_cx: &'a FinalizeContext<'a>,
/// The span of the attribute currently being parsed
pub(crate) attr_span: Span,
pub(crate) attr_style: AttrStyle,
}

impl<'a> AcceptContext<'a> {
Expand Down Expand Up @@ -269,6 +272,7 @@ impl<'sess> AttributeParser<'sess> {
let cx = AcceptContext {
group_cx: &group_cx,
attr_span: lower_span(attr.span),
attr_style: attr.style,
};

f(&cx, &args)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_attr_parsing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub mod parser;
mod session_diagnostics;

pub use attributes::cfg::*;
pub use attributes::util::{find_crate_name, is_builtin_attr, parse_version};
pub use attributes::util::{is_builtin_attr, parse_version};
pub use context::{AttributeParser, OmitDoc};
pub use rustc_attr_data_structures::*;

Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_attr_parsing/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,3 +479,11 @@ pub(crate) struct UnrecognizedReprHint {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_name_value, code = E0539)]
pub(crate) struct ExpectedNameValue {
#[primary_span]
pub span: Span,
pub name: Symbol,
}
20 changes: 17 additions & 3 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::sync::{Arc, LazyLock};
use std::{env, fs, iter};

use rustc_ast as ast;
use rustc_attr_parsing::{AttributeKind, AttributeParser};
use rustc_codegen_ssa::traits::CodegenBackend;
use rustc_data_structures::parallel;
use rustc_data_structures::steal::Steal;
Expand All @@ -32,7 +33,7 @@ use rustc_session::output::{collect_crate_types, filename_for_input};
use rustc_session::search_paths::PathKind;
use rustc_session::{Limit, Session};
use rustc_span::{
ErrorGuaranteed, FileName, SourceFileHash, SourceFileHashAlgorithm, Span, Symbol, sym,
DUMMY_SP, ErrorGuaranteed, FileName, SourceFileHash, SourceFileHashAlgorithm, Span, Symbol, sym,
};
use rustc_target::spec::PanicStrategy;
use rustc_trait_selection::traits;
Expand Down Expand Up @@ -1119,6 +1120,20 @@ pub(crate) fn start_codegen<'tcx>(
codegen
}

pub(crate) fn parse_crate_name(
sess: &Session,
attrs: &[ast::Attribute],
limit_diagnostics: bool,
) -> Option<(Symbol, Span)> {
let rustc_hir::Attribute::Parsed(AttributeKind::CrateName { name, name_span, .. }) =
AttributeParser::parse_limited(sess, &attrs, sym::crate_name, DUMMY_SP, limit_diagnostics)?
else {
unreachable!("crate_name is the only attr we could've parsed here");
};

Some((name, name_span))
}

/// Compute and validate the crate name.
pub fn get_crate_name(sess: &Session, krate_attrs: &[ast::Attribute]) -> Symbol {
// We validate *all* occurrences of `#![crate_name]`, pick the first find and
Expand All @@ -1128,8 +1143,7 @@ pub fn get_crate_name(sess: &Session, krate_attrs: &[ast::Attribute]) -> Symbol
// in all code paths that require the crate name very early on, namely before
// macro expansion.

let attr_crate_name =
validate_and_find_value_str_builtin_attr(sym::crate_name, sess, krate_attrs);
let attr_crate_name = parse_crate_name(sess, krate_attrs, false);

let validate = |name, span| {
rustc_session::output::validate_crate_name(sess, name, span);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use rustc_target::spec::Target;
use tracing::info;

use crate::errors;
use crate::passes::parse_crate_name;

/// Function pointer type that constructs a new CodegenBackend.
type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
Expand Down Expand Up @@ -485,7 +486,7 @@ pub fn build_output_filenames(attrs: &[ast::Attribute], sess: &Session) -> Outpu
.opts
.crate_name
.clone()
.or_else(|| rustc_attr_parsing::find_crate_name(attrs).map(|n| n.to_string()));
.or_else(|| parse_crate_name(sess, attrs, true).map(|i| i.0.to_string()));

match sess.io.output_file {
None => {
Expand Down
54 changes: 23 additions & 31 deletions compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use rustc_abi::ExternAbi;
use rustc_attr_parsing::{AttributeKind, AttributeParser, ReprAttr};
use rustc_attr_parsing::{AttributeKind, AttributeParser, ReprAttr, find_attr};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::FnKind;
use rustc_hir::{AttrArgs, AttrItem, Attribute, GenericParamKind, PatExprKind, PatKind};
use rustc_hir::{Attribute, GenericParamKind, PatExprKind, PatKind};
use rustc_middle::ty;
use rustc_session::config::CrateType;
use rustc_session::{declare_lint, declare_lint_pass};
Expand Down Expand Up @@ -342,35 +342,27 @@ impl<'tcx> LateLintPass<'tcx> for NonSnakeCase {
let crate_ident = if let Some(name) = &cx.tcx.sess.opts.crate_name {
Some(Ident::from_str(name))
} else {
ast::attr::find_by_name(cx.tcx.hir().attrs(hir::CRATE_HIR_ID), sym::crate_name)
.and_then(|attr| {
if let Attribute::Unparsed(n) = attr
&& let AttrItem { args: AttrArgs::Eq { eq_span: _, expr: lit }, .. } =
n.as_ref()
&& let ast::LitKind::Str(name, ..) = lit.kind
{
// Discard the double quotes surrounding the literal.
let sp = cx
.sess()
.source_map()
.span_to_snippet(lit.span)
.ok()
.and_then(|snippet| {
let left = snippet.find('"')?;
let right = snippet.rfind('"').map(|pos| snippet.len() - pos)?;

Some(
lit.span
.with_lo(lit.span.lo() + BytePos(left as u32 + 1))
.with_hi(lit.span.hi() - BytePos(right as u32)),
)
})
.unwrap_or(lit.span);

Some(Ident::new(name, sp))
} else {
None
}
find_attr!(cx.tcx.hir().attrs(hir::CRATE_HIR_ID), AttributeKind::CrateName{name, name_span,..} => (name, name_span))
.and_then(|(&name, &span)| {
// Discard the double quotes surrounding the literal.
let sp = cx
.sess()
.source_map()
.span_to_snippet(span)
.ok()
.and_then(|snippet| {
let left = snippet.find('"')?;
let right = snippet.rfind('"').map(|pos| snippet.len() - pos)?;

Some(
span
.with_lo(span.lo() + BytePos(left as u32 + 1))
.with_hi(span.hi() - BytePos(right as u32)),
)
})
.unwrap_or(span);

Some(Ident::new(name, sp))
})
};

Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_parse/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ pub fn check_builtin_meta_item(
) {
// Some special attributes like `cfg` must be checked
// before the generic check, so we skip them here.
let should_skip = |name| name == sym::cfg;
//
// Also, attributes that have a new-style parser don't need to be checked here anymore
let should_skip = |name| name == sym::cfg || name == sym::crate_name;

if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
emit_malformed_attribute(psess, style, meta.span, name, template);
Expand Down
53 changes: 37 additions & 16 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,22 +315,43 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
let builtin = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));

if hir_id != CRATE_HIR_ID {
if let Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) =
attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name))
{
match attr.style() {
ast::AttrStyle::Outer => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span(),
errors::OuterCrateLevelAttr,
),
ast::AttrStyle::Inner => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span(),
errors::InnerCrateLevelAttr,
),
if let Attribute::Parsed(attr) = attr {
if let Some((style, span)) = attr.crate_level() {
match style {
ast::AttrStyle::Outer => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
span,
errors::OuterCrateLevelAttr,
),
ast::AttrStyle::Inner => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
span,
errors::InnerCrateLevelAttr,
),
}
}
} else {
// FIXME(jdonszelmann): remove once all crate-level attrs are parsed and caught by
// the above
if let Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) =
attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name))
{
match attr.style() {
ast::AttrStyle::Outer => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span(),
errors::OuterCrateLevelAttr,
),
ast::AttrStyle::Inner => self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span(),
errors::InnerCrateLevelAttr,
),
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/attributes/crate-name-macro-call.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// issue: rust-lang/rust#122001
// Ensure we reject macro calls inside `#![crate_name]` as their result wouldn't get honored anyway.

#![crate_name = concat!("my", "crate")] //~ ERROR malformed `crate_name` attribute input
#![crate_name = concat!("my", "crate")] //~ ERROR expected a quoted string literal

fn main() {}
Loading
Loading