Skip to content

Commit 2fef0a3

Browse files
nnethercotennethercote
committed
Replace infallible name_or_empty methods with fallible name methods.
I'm removing empty identifiers everywhere, because in practice they always mean "no identifier" rather than "empty identifier". (An empty identifier is impossible.) It's better to use `Option` to mean "no identifier" because you then can't forget about the "no identifier" possibility. Some specifics: - When testing an attribute for a single name, the commit uses the `has_name` method. - When testing an attribute for multiple names, the commit uses the new `has_any_name` method. - When using `match` on an attribute, the match arms now have `Some` on them. In the tests, we now avoid printing empty identifiers by not printing the identifier in the `error:` line at all, instead letting the carets point out the problem.
1 parent 7e1f2f9 commit 2fef0a3

File tree

40 files changed

+217
-203
lines changed

40 files changed

+217
-203
lines changed

Diff for: ‎compiler/rustc_ast/src/attr/mod.rs

+22-11
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ impl MetaItem {
305305
if let [PathSegment { ident, .. }] = self.path.segments[..] { Some(ident) } else { None }
306306
}
307307

308-
pub fn name_or_empty(&self) -> Symbol {
309-
self.ident().unwrap_or_else(Ident::empty).name
308+
pub fn name(&self) -> Option<Symbol> {
309+
self.ident().map(|ident| ident.name)
310310
}
311311

312312
pub fn has_name(&self, name: Symbol) -> bool {
@@ -511,13 +511,14 @@ impl MetaItemInner {
511511
}
512512
}
513513

514-
/// For a single-segment meta item, returns its name; otherwise, returns `None`.
514+
/// For a single-segment meta item, returns its identifier; otherwise, returns `None`.
515515
pub fn ident(&self) -> Option<Ident> {
516516
self.meta_item().and_then(|meta_item| meta_item.ident())
517517
}
518518

519-
pub fn name_or_empty(&self) -> Symbol {
520-
self.ident().unwrap_or_else(Ident::empty).name
519+
/// For a single-segment meta item, returns its name; otherwise, returns `None`.
520+
pub fn name(&self) -> Option<Symbol> {
521+
self.ident().map(|ident| ident.name)
521522
}
522523

523524
/// Returns `true` if this list item is a MetaItem with a name of `name`.
@@ -738,9 +739,9 @@ pub trait AttributeExt: Debug {
738739
fn id(&self) -> AttrId;
739740

740741
/// For a single-segment attribute (i.e., `#[attr]` and not `#[path::atrr]`),
741-
/// return the name of the attribute, else return the empty identifier.
742-
fn name_or_empty(&self) -> Symbol {
743-
self.ident().unwrap_or_else(Ident::empty).name
742+
/// return the name of the attribute; otherwise, returns `None`.
743+
fn name(&self) -> Option<Symbol> {
744+
self.ident().map(|ident| ident.name)
744745
}
745746

746747
/// Get the meta item list, `#[attr(meta item list)]`
@@ -752,7 +753,7 @@ pub trait AttributeExt: Debug {
752753
/// Gets the span of the value literal, as string, when using `#[attr = value]`
753754
fn value_span(&self) -> Option<Span>;
754755

755-
/// For a single-segment attribute, returns its name; otherwise, returns `None`.
756+
/// For a single-segment attribute, returns its ident; otherwise, returns `None`.
756757
fn ident(&self) -> Option<Ident>;
757758

758759
/// Checks whether the path of this attribute matches the name.
@@ -770,6 +771,11 @@ pub trait AttributeExt: Debug {
770771
self.ident().map(|x| x.name == name).unwrap_or(false)
771772
}
772773

774+
#[inline]
775+
fn has_any_name(&self, names: &[Symbol]) -> bool {
776+
names.iter().any(|&name| self.has_name(name))
777+
}
778+
773779
/// get the span of the entire attribute
774780
fn span(&self) -> Span;
775781

@@ -813,8 +819,8 @@ impl Attribute {
813819
AttributeExt::id(self)
814820
}
815821

816-
pub fn name_or_empty(&self) -> Symbol {
817-
AttributeExt::name_or_empty(self)
822+
pub fn name(&self) -> Option<Symbol> {
823+
AttributeExt::name(self)
818824
}
819825

820826
pub fn meta_item_list(&self) -> Option<ThinVec<MetaItemInner>> {
@@ -846,6 +852,11 @@ impl Attribute {
846852
AttributeExt::has_name(self, name)
847853
}
848854

855+
#[inline]
856+
pub fn has_any_name(&self, names: &[Symbol]) -> bool {
857+
AttributeExt::has_any_name(self, names)
858+
}
859+
849860
pub fn span(&self) -> Span {
850861
AttributeExt::span(self)
851862
}

Diff for: ‎compiler/rustc_ast_lowering/src/item.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1310,7 +1310,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13101310
// create a fake body so that the entire rest of the compiler doesn't have to deal with
13111311
// this as a special case.
13121312
return self.lower_fn_body(decl, contract, |this| {
1313-
if attrs.iter().any(|a| a.name_or_empty() == sym::rustc_intrinsic) {
1313+
if attrs.iter().any(|a| a.has_name(sym::rustc_intrinsic)) {
13141314
let span = this.lower_span(span);
13151315
let empty_block = hir::Block {
13161316
hir_id: this.next_id(),

Diff for: ‎compiler/rustc_ast_passes/src/ast_validation.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ impl<'a> AstValidator<'a> {
347347
sym::forbid,
348348
sym::warn,
349349
];
350-
!arr.contains(&attr.name_or_empty()) && rustc_attr_parsing::is_builtin_attr(*attr)
350+
!attr.has_any_name(&arr) && rustc_attr_parsing::is_builtin_attr(*attr)
351351
})
352352
.for_each(|attr| {
353353
if attr.is_doc_comment() {
@@ -945,8 +945,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
945945
self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
946946
self.check_defaultness(item.span, *defaultness);
947947

948-
let is_intrinsic =
949-
item.attrs.iter().any(|a| a.name_or_empty() == sym::rustc_intrinsic);
948+
let is_intrinsic = item.attrs.iter().any(|a| a.has_name(sym::rustc_intrinsic));
950949
if body.is_none() && !is_intrinsic {
951950
self.dcx().emit_err(errors::FnWithoutBody {
952951
span: item.span,

Diff for: ‎compiler/rustc_attr_parsing/src/attributes/cfg.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ pub fn eval_condition(
117117
};
118118

119119
match &cfg.kind {
120-
MetaItemKind::List(mis) if cfg.name_or_empty() == sym::version => {
120+
MetaItemKind::List(mis) if cfg.has_name(sym::version) => {
121121
try_gate_cfg(sym::version, cfg.span, sess, features);
122122
let (min_version, span) = match &mis[..] {
123123
[MetaItemInner::Lit(MetaItemLit { kind: LitKind::Str(sym, ..), span, .. })] => {
@@ -164,26 +164,26 @@ pub fn eval_condition(
164164

165165
// The unwraps below may look dangerous, but we've already asserted
166166
// that they won't fail with the loop above.
167-
match cfg.name_or_empty() {
168-
sym::any => mis
167+
match cfg.name() {
168+
Some(sym::any) => mis
169169
.iter()
170170
// We don't use any() here, because we want to evaluate all cfg condition
171171
// as eval_condition can (and does) extra checks
172172
.fold(false, |res, mi| res | eval_condition(mi, sess, features, eval)),
173-
sym::all => mis
173+
Some(sym::all) => mis
174174
.iter()
175175
// We don't use all() here, because we want to evaluate all cfg condition
176176
// as eval_condition can (and does) extra checks
177177
.fold(true, |res, mi| res & eval_condition(mi, sess, features, eval)),
178-
sym::not => {
178+
Some(sym::not) => {
179179
let [mi] = mis.as_slice() else {
180180
dcx.emit_err(session_diagnostics::ExpectedOneCfgPattern { span: cfg.span });
181181
return false;
182182
};
183183

184184
!eval_condition(mi, sess, features, eval)
185185
}
186-
sym::target => {
186+
Some(sym::target) => {
187187
if let Some(features) = features
188188
&& !features.cfg_target_compact()
189189
{

Diff for: ‎compiler/rustc_attr_parsing/src/context.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ impl<'sess> AttributeParser<'sess> {
222222
// if we're only looking for a single attribute,
223223
// skip all the ones we don't care about
224224
if let Some(expected) = self.parse_only {
225-
if attr.name_or_empty() != expected {
225+
if !attr.has_name(expected) {
226226
continue;
227227
}
228228
}
@@ -232,7 +232,7 @@ impl<'sess> AttributeParser<'sess> {
232232
// that's expanded right? But no, sometimes, when parsing attributes on macros,
233233
// we already use the lowering logic and these are still there. So, when `omit_doc`
234234
// is set we *also* want to ignore these
235-
if omit_doc == OmitDoc::Skip && attr.name_or_empty() == sym::doc {
235+
if omit_doc == OmitDoc::Skip && attr.has_name(sym::doc) {
236236
continue;
237237
}
238238

@@ -250,7 +250,7 @@ impl<'sess> AttributeParser<'sess> {
250250
}))
251251
}
252252
// // FIXME: make doc attributes go through a proper attribute parser
253-
// ast::AttrKind::Normal(n) if n.name_or_empty() == sym::doc => {
253+
// ast::AttrKind::Normal(n) if n.has_name(sym::doc) => {
254254
// let p = GenericMetaItemParser::from_attr(&n, self.dcx());
255255
//
256256
// attributes.push(Attribute::Parsed(AttributeKind::DocComment {

Diff for: ‎compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -527,15 +527,14 @@ impl<'a> TraitDef<'a> {
527527
item.attrs
528528
.iter()
529529
.filter(|a| {
530-
[
530+
a.has_any_name(&[
531531
sym::allow,
532532
sym::warn,
533533
sym::deny,
534534
sym::forbid,
535535
sym::stable,
536536
sym::unstable,
537-
]
538-
.contains(&a.name_or_empty())
537+
])
539538
})
540539
.cloned(),
541540
);

Diff for: ‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

+19-14
Original file line numberDiff line numberDiff line change
@@ -345,20 +345,26 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
345345
no_sanitize_span = Some(attr.span());
346346
if let Some(list) = attr.meta_item_list() {
347347
for item in list.iter() {
348-
match item.name_or_empty() {
349-
sym::address => {
348+
match item.name() {
349+
Some(sym::address) => {
350350
codegen_fn_attrs.no_sanitize |=
351351
SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS
352352
}
353-
sym::cfi => codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI,
354-
sym::kcfi => codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI,
355-
sym::memory => codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY,
356-
sym::memtag => codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG,
357-
sym::shadow_call_stack => {
353+
Some(sym::cfi) => codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI,
354+
Some(sym::kcfi) => codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI,
355+
Some(sym::memory) => {
356+
codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY
357+
}
358+
Some(sym::memtag) => {
359+
codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG
360+
}
361+
Some(sym::shadow_call_stack) => {
358362
codegen_fn_attrs.no_sanitize |= SanitizerSet::SHADOWCALLSTACK
359363
}
360-
sym::thread => codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD,
361-
sym::hwaddress => {
364+
Some(sym::thread) => {
365+
codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD
366+
}
367+
Some(sym::hwaddress) => {
362368
codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS
363369
}
364370
_ => {
@@ -419,9 +425,9 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
419425
continue;
420426
};
421427

422-
let attrib_to_write = match meta_item.name_or_empty() {
423-
sym::prefix_nops => &mut prefix,
424-
sym::entry_nops => &mut entry,
428+
let attrib_to_write = match meta_item.name() {
429+
Some(sym::prefix_nops) => &mut prefix,
430+
Some(sym::entry_nops) => &mut entry,
425431
_ => {
426432
tcx.dcx().emit_err(errors::UnexpectedParameterName {
427433
span: item.span(),
@@ -785,8 +791,7 @@ impl<'a> MixedExportNameAndNoMangleState<'a> {
785791
fn autodiff_attrs(tcx: TyCtxt<'_>, id: DefId) -> Option<AutoDiffAttrs> {
786792
let attrs = tcx.get_attrs(id, sym::rustc_autodiff);
787793

788-
let attrs =
789-
attrs.filter(|attr| attr.name_or_empty() == sym::rustc_autodiff).collect::<Vec<_>>();
794+
let attrs = attrs.filter(|attr| attr.has_name(sym::rustc_autodiff)).collect::<Vec<_>>();
790795

791796
// check for exactly one autodiff attribute on placeholder functions.
792797
// There should only be one, since we generate a new placeholder per ad macro.

Diff for: ‎compiler/rustc_expand/src/base.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -824,10 +824,10 @@ impl SyntaxExtension {
824824
return Err(item.span);
825825
}
826826

827-
match item.name_or_empty() {
828-
sym::no => Ok(CollapseMacroDebuginfo::No),
829-
sym::external => Ok(CollapseMacroDebuginfo::External),
830-
sym::yes => Ok(CollapseMacroDebuginfo::Yes),
827+
match item.name() {
828+
Some(sym::no) => Ok(CollapseMacroDebuginfo::No),
829+
Some(sym::external) => Ok(CollapseMacroDebuginfo::External),
830+
Some(sym::yes) => Ok(CollapseMacroDebuginfo::Yes),
831831
_ => Err(item.path.span),
832832
}
833833
}

Diff for: ‎compiler/rustc_expand/src/expand.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -2053,8 +2053,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
20532053
) -> Node::OutputTy {
20542054
loop {
20552055
return match self.take_first_attr(&mut node) {
2056-
Some((attr, pos, derives)) => match attr.name_or_empty() {
2057-
sym::cfg => {
2056+
Some((attr, pos, derives)) => match attr.name() {
2057+
Some(sym::cfg) => {
20582058
let (res, meta_item) = self.expand_cfg_true(&mut node, attr, pos);
20592059
if res {
20602060
continue;
@@ -2071,7 +2071,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
20712071
}
20722072
Default::default()
20732073
}
2074-
sym::cfg_attr => {
2074+
Some(sym::cfg_attr) => {
20752075
self.expand_cfg_attr(&mut node, &attr, pos);
20762076
continue;
20772077
}
@@ -2144,8 +2144,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
21442144
) {
21452145
loop {
21462146
return match self.take_first_attr(node) {
2147-
Some((attr, pos, derives)) => match attr.name_or_empty() {
2148-
sym::cfg => {
2147+
Some((attr, pos, derives)) => match attr.name() {
2148+
Some(sym::cfg) => {
21492149
let span = attr.span;
21502150
if self.expand_cfg_true(node, attr, pos).0 {
21512151
continue;
@@ -2154,7 +2154,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
21542154
node.expand_cfg_false(self, pos, span);
21552155
continue;
21562156
}
2157-
sym::cfg_attr => {
2157+
Some(sym::cfg_attr) => {
21582158
self.expand_cfg_attr(node, &attr, pos);
21592159
continue;
21602160
}

Diff for: ‎compiler/rustc_hir/src/hir.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -1237,7 +1237,7 @@ impl AttributeExt for Attribute {
12371237
Attribute::Parsed(AttributeKind::DocComment { kind, comment, .. }) => {
12381238
Some((*comment, *kind))
12391239
}
1240-
Attribute::Unparsed(_) if self.name_or_empty() == sym::doc => {
1240+
Attribute::Unparsed(_) if self.has_name(sym::doc) => {
12411241
self.value_str().map(|s| (s, CommentKind::Line))
12421242
}
12431243
_ => None,
@@ -1262,8 +1262,8 @@ impl Attribute {
12621262
}
12631263

12641264
#[inline]
1265-
pub fn name_or_empty(&self) -> Symbol {
1266-
AttributeExt::name_or_empty(self)
1265+
pub fn name(&self) -> Option<Symbol> {
1266+
AttributeExt::name(self)
12671267
}
12681268

12691269
#[inline]
@@ -1301,6 +1301,11 @@ impl Attribute {
13011301
AttributeExt::has_name(self, name)
13021302
}
13031303

1304+
#[inline]
1305+
pub fn has_any_name(&self, names: &[Symbol]) -> bool {
1306+
AttributeExt::has_any_name(self, names)
1307+
}
1308+
13041309
#[inline]
13051310
pub fn span(&self) -> Span {
13061311
AttributeExt::span(self)

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ fn parse_never_type_options_attr(
488488
item.span(),
489489
format!(
490490
"unknown or duplicate never type option: `{}` (supported: `fallback`, `diverging_block_default`)",
491-
item.name_or_empty()
491+
item.name().unwrap()
492492
),
493493
);
494494
}

Diff for: ‎compiler/rustc_hir_typeck/src/method/probe.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -2334,8 +2334,9 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
23342334
let hir_id = self.fcx.tcx.local_def_id_to_hir_id(local_def_id);
23352335
let attrs = self.fcx.tcx.hir_attrs(hir_id);
23362336
for attr in attrs {
2337-
if sym::doc == attr.name_or_empty() {
2338-
} else if sym::rustc_confusables == attr.name_or_empty() {
2337+
if attr.has_name(sym::doc) {
2338+
// do nothing
2339+
} else if attr.has_name(sym::rustc_confusables) {
23392340
let Some(confusables) = attr.meta_item_list() else {
23402341
continue;
23412342
};
@@ -2355,7 +2356,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
23552356
continue;
23562357
};
23572358
for v in values {
2358-
if v.name_or_empty() != sym::alias {
2359+
if !v.has_name(sym::alias) {
23592360
continue;
23602361
}
23612362
if let Some(nested) = v.meta_item_list() {

Diff for: ‎compiler/rustc_incremental/messages.ftl

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ incremental_undefined_clean_dirty_assertions =
9393
incremental_undefined_clean_dirty_assertions_item =
9494
clean/dirty auto-assertions not yet defined for Node::Item.node={$kind}
9595
96-
incremental_unknown_item = unknown item `{$name}`
96+
incremental_unknown_rustc_clean_argument = unknown `rustc_clean` argument
9797
9898
incremental_unrecognized_depnode = unrecognized `DepNode` variant: {$name}
9999

Diff for: ‎compiler/rustc_incremental/src/errors.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,10 @@ pub(crate) struct NotLoaded<'a> {
107107
}
108108

109109
#[derive(Diagnostic)]
110-
#[diag(incremental_unknown_item)]
111-
pub(crate) struct UnknownItem {
110+
#[diag(incremental_unknown_rustc_clean_argument)]
111+
pub(crate) struct UnknownRustcCleanArgument {
112112
#[primary_span]
113113
pub span: Span,
114-
pub name: Symbol,
115114
}
116115

117116
#[derive(Diagnostic)]

0 commit comments

Comments
 (0)