Skip to content

Reject invalid literal suffixes in tuple indexing, tuple struct indexing, and struct field name position #145463

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

Open
wants to merge 4 commits 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
3 changes: 0 additions & 3 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,6 @@ parse_invalid_label =
parse_invalid_literal_suffix_on_tuple_index = suffixes on a tuple index are invalid
.label = invalid suffix `{$suffix}`
.tuple_exception_line_1 = `{$suffix}` is *temporarily* accepted on tuple index fields as it was incorrectly accepted on stable for a few releases
.tuple_exception_line_2 = on proc macros, you'll want to use `syn::Index::from` or `proc_macro::Literal::*_unsuffixed` for code that will desugar to tuple field access
.tuple_exception_line_3 = see issue #60210 <https://github.com/rust-lang/rust/issues/60210> for more information
parse_invalid_logical_operator = `{$incorrect}` is not a logical operator
.note = unlike in e.g., Python and PHP, `&&` and `||` are used for logical operators
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1016,10 +1016,6 @@ pub(crate) struct InvalidLiteralSuffixOnTupleIndex {
#[label]
pub span: Span,
pub suffix: Symbol,
#[help(parse_tuple_exception_line_1)]
#[help(parse_tuple_exception_line_2)]
#[help(parse_tuple_exception_line_3)]
pub exception: bool,
}

#[derive(Diagnostic)]
Expand Down
26 changes: 6 additions & 20 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,10 @@ impl<'a> Parser<'a> {
suffix,
}) => {
if let Some(suffix) = suffix {
self.expect_no_tuple_index_suffix(current.span, suffix);
self.dcx().emit_err(errors::InvalidLiteralSuffixOnTupleIndex {
span: current.span,
suffix,
});
}
match self.break_up_float(symbol, current.span) {
// 1e2
Expand Down Expand Up @@ -1239,7 +1242,8 @@ impl<'a> Parser<'a> {
suffix: Option<Symbol>,
) -> Box<Expr> {
if let Some(suffix) = suffix {
self.expect_no_tuple_index_suffix(ident_span, suffix);
self.dcx()
.emit_err(errors::InvalidLiteralSuffixOnTupleIndex { span: ident_span, suffix });
}
self.mk_expr(lo.to(ident_span), ExprKind::Field(base, Ident::new(field, ident_span)))
}
Expand Down Expand Up @@ -2225,24 +2229,6 @@ impl<'a> Parser<'a> {
})
}

pub(super) fn expect_no_tuple_index_suffix(&self, span: Span, suffix: Symbol) {
if [sym::i32, sym::u32, sym::isize, sym::usize].contains(&suffix) {
// #59553: warn instead of reject out of hand to allow the fix to percolate
// through the ecosystem when people fix their macros
self.dcx().emit_warn(errors::InvalidLiteralSuffixOnTupleIndex {
span,
suffix,
exception: true,
});
} else {
self.dcx().emit_err(errors::InvalidLiteralSuffixOnTupleIndex {
span,
suffix,
exception: false,
});
}
}

/// Matches `'-' lit | lit` (cf. `ast_validation::AstValidator::check_expr_within_pat`).
/// Keep this in sync with `Token::can_begin_literal_maybe_minus`.
pub fn parse_literal_maybe_minus(&mut self) -> PResult<'a, Box<Expr>> {
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,10 @@ impl<'a> Parser<'a> {
if let token::Literal(token::Lit { kind: token::Integer, symbol, suffix }) = self.token.kind
{
if let Some(suffix) = suffix {
self.expect_no_tuple_index_suffix(self.token.span, suffix);
self.dcx().emit_err(errors::InvalidLiteralSuffixOnTupleIndex {
span: self.token.span,
suffix,
});
}
self.bump();
Ok(Ident::new(symbol, self.prev_token.span))
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2021,7 +2021,6 @@ ui/parser/issues/issue-5806.rs
ui/parser/issues/issue-58094-missing-right-square-bracket.rs
ui/parser/issues/issue-58856-1.rs
ui/parser/issues/issue-58856-2.rs
ui/parser/issues/issue-59418.rs
ui/parser/issues/issue-60075.rs
ui/parser/issues/issue-61858.rs
ui/parser/issues/issue-62524.rs
Expand Down
33 changes: 33 additions & 0 deletions tests/ui/parser/auxiliary/tuple-index-suffix-proc-macro-aux.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#![feature(proc_macro_quote, proc_macro_span)]

extern crate proc_macro;

use proc_macro::{Ident, Literal, Span, TokenStream, TokenTree, quote};

#[proc_macro]
pub fn bad_tup_indexing(input: TokenStream) -> TokenStream {
let tt = input.into_iter().next().unwrap();
let TokenTree::Literal(indexing_expr) = tt else {
unreachable!();
};
quote! { (42,).$indexing_expr }
}

// Expects {IDENT, COMMA, LITERAL}
#[proc_macro]
pub fn bad_tup_struct_indexing(input: TokenStream) -> TokenStream {
let mut input = input.into_iter();

let ident = input.next().unwrap();
let _comma = input.next().unwrap();
let lit = input.next().unwrap();

let TokenTree::Ident(ident) = ident else {
unreachable!("id");
};
let TokenTree::Literal(indexing_expr) = lit else {
unreachable!("lit");
};

quote! { $ident.$indexing_expr }
}
18 changes: 0 additions & 18 deletions tests/ui/parser/issues/issue-59418.rs

This file was deleted.

26 changes: 0 additions & 26 deletions tests/ui/parser/issues/issue-59418.stderr

This file was deleted.

32 changes: 32 additions & 0 deletions tests/ui/parser/tuple-index-suffix-proc-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//! See #59418.
//!
//! Like `tuple-index-suffix.rs`, but exercises the proc-macro interaction.

//@ proc-macro: tuple-index-suffix-proc-macro-aux.rs

extern crate tuple_index_suffix_proc_macro_aux;
use tuple_index_suffix_proc_macro_aux as aux;

fn main() {
struct TupStruct(i32);
let tup_struct = TupStruct(42);

// Previously, #60186 had carve outs for `{i,u}{32,usize}` as non-lint pseudo-FCW warnings. Now,
// they all hard error.

aux::bad_tup_indexing!(0usize);
//~^ ERROR suffixes on a tuple index are invalid
aux::bad_tup_struct_indexing!(tup_struct, 0isize);
//~^ ERROR suffixes on a tuple index are invalid

// Not part of the #60186 carve outs.

aux::bad_tup_indexing!(0u8);
//~^ ERROR suffixes on a tuple index are invalid
aux::bad_tup_struct_indexing!(tup_struct, 0u64);
//~^ ERROR suffixes on a tuple index are invalid

// NOTE: didn't bother with trying to figure out how to generate `struct P { 0u32: u32 }` using
// *only* `proc_macro` without help with `syn`/`quote`, looks like you can't with just
// `proc_macro::quote`?
}
26 changes: 26 additions & 0 deletions tests/ui/parser/tuple-index-suffix-proc-macro.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: suffixes on a tuple index are invalid
--> $DIR/tuple-index-suffix-proc-macro.rs:17:28
|
LL | aux::bad_tup_indexing!(0usize);
| ^^^^^^ invalid suffix `usize`

error: suffixes on a tuple index are invalid
--> $DIR/tuple-index-suffix-proc-macro.rs:19:47
|
LL | aux::bad_tup_struct_indexing!(tup_struct, 0isize);
| ^^^^^^ invalid suffix `isize`

error: suffixes on a tuple index are invalid
--> $DIR/tuple-index-suffix-proc-macro.rs:24:28
|
LL | aux::bad_tup_indexing!(0u8);
| ^^^ invalid suffix `u8`

error: suffixes on a tuple index are invalid
--> $DIR/tuple-index-suffix-proc-macro.rs:26:47
|
LL | aux::bad_tup_struct_indexing!(tup_struct, 0u64);
| ^^^^ invalid suffix `u64`

error: aborting due to 4 previous errors

79 changes: 79 additions & 0 deletions tests/ui/parser/tuple-index-suffix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
//! Regression test for both the original regression in #59418 where invalid suffixes in indexing
//! positions were accidentally accepted, and also for the removal of the temporary carve out that
//! mitigated ecosystem impact following trying to reject #59418 (this was implemented as a FCW
//! tracked in #60210).
//!
//! Check that we hard error on invalid suffixes in tuple indexing subexpressions and struct numeral
//! field names.

struct X(i32,i32,i32);

fn main() {
let tup_struct = X(1, 2, 3);
let invalid_tup_struct_suffix = tup_struct.0suffix;
//~^ ERROR suffixes on a tuple index are invalid
let previous_carve_out_tup_struct_suffix = tup_struct.0i32;
//~^ ERROR suffixes on a tuple index are invalid

let tup = (1, 2, 3);
let invalid_tup_suffix = tup.0suffix;
//~^ ERROR suffixes on a tuple index are invalid
let previous_carve_out_tup_suffix = tup.0u32;
//~^ ERROR suffixes on a tuple index are invalid

numeral_struct_field_name_suffix_invalid();
numeral_struct_field_name_suffix_previous_carve_out();
}

// Previously, there were very limited carve outs as a ecosystem impact mitigation implemented in
// #60186. *Only* `{i,u}{32,usize}` suffixes were temporarily accepted. Now, they all hard error.
fn previous_carve_outs() {
// Previously temporarily accepted by a pseudo-FCW (#60210), now hard error.

let previous_carve_out_i32 = (42,).0i32; //~ ERROR suffixes on a tuple index are invalid
let previous_carve_out_i32 = (42,).0u32; //~ ERROR suffixes on a tuple index are invalid
let previous_carve_out_isize = (42,).0isize; //~ ERROR suffixes on a tuple index are invalid
let previous_carve_out_usize = (42,).0usize; //~ ERROR suffixes on a tuple index are invalid

// Not part of the carve outs!
let error_i8 = (42,).0i8; //~ ERROR suffixes on a tuple index are invalid
let error_u8 = (42,).0u8; //~ ERROR suffixes on a tuple index are invalid
let error_i16 = (42,).0i16; //~ ERROR suffixes on a tuple index are invalid
let error_u16 = (42,).0u16; //~ ERROR suffixes on a tuple index are invalid
let error_i64 = (42,).0i64; //~ ERROR suffixes on a tuple index are invalid
let error_u64 = (42,).0u64; //~ ERROR suffixes on a tuple index are invalid
let error_i128 = (42,).0i128; //~ ERROR suffixes on a tuple index are invalid
let error_u128 = (42,).0u128; //~ ERROR suffixes on a tuple index are invalid
}

fn numeral_struct_field_name_suffix_invalid() {
let invalid_struct_name = X { 0suffix: 0, 1: 1, 2: 2 };
//~^ ERROR suffixes on a tuple index are invalid
match invalid_struct_name {
X { 0suffix: _, .. } => {}
//~^ ERROR suffixes on a tuple index are invalid
}
}

fn numeral_struct_field_name_suffix_previous_carve_out() {
let carve_out_struct_name = X { 0u32: 0, 1: 1, 2: 2 };
//~^ ERROR suffixes on a tuple index are invalid
match carve_out_struct_name {
X { 0u32: _, .. } => {}
//~^ ERROR suffixes on a tuple index are invalid
}
}

// Unfortunately, it turns out `std::mem::offset_of!` uses the same expect suffix code path.
fn offset_of_suffix() {
#[repr(C)]
pub struct Struct<T>(u8, T);

// Previous pseudo-FCW carve outs
assert_eq!(std::mem::offset_of!(Struct<u32>, 0usize), 0);
//~^ ERROR suffixes on a tuple index are invalid

// Not part of carve outs
assert_eq!(std::mem::offset_of!(Struct<u32>, 0u8), 0);
//~^ ERROR suffixes on a tuple index are invalid
}
Loading
Loading