Skip to content

Commit

Permalink
Unrolled build for rust-lang#134599
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#134599 - dtolnay:fulldepsparser, r=fmease

Detect invalid exprs in parser used by pretty-printer tests

This PR fixes a bug in rust-lang#133730 inherited from rust-lang#43742. Before this fix, the test might silently only operate on a prefix of some of the test cases in this table:

https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/tests/ui-fulldeps/pprust-parenthesis-insertion.rs#L57

For example, adding the test case `1 .. 2 .. 3` (a syntactically invalid expression) into the table would unexpectedly succeed the test instead of crashing at this unwrap:

https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/tests/ui-fulldeps/pprust-parenthesis-insertion.rs#L199-L200

because `parse_expr` would successfully parse just `1 .. 2` and disregard the last `.. 3`.

This PR adds a check that `parse_expr` reaches `Eof`, ensuring all the test cases actually test the whole expression they look like they are supposed to.
  • Loading branch information
rust-timer authored Dec 22, 2024
2 parents a2bcfae + 1f2028f commit 4cb124d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 50 deletions.
51 changes: 51 additions & 0 deletions tests/ui-fulldeps/auxiliary/parser.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#![feature(rustc_private)]

extern crate rustc_ast;
extern crate rustc_driver;
extern crate rustc_errors;
extern crate rustc_parse;
extern crate rustc_session;
extern crate rustc_span;

use rustc_ast::ast::{DUMMY_NODE_ID, Expr};
use rustc_ast::mut_visit::MutVisitor;
use rustc_ast::node_id::NodeId;
use rustc_ast::ptr::P;
use rustc_ast::token;
use rustc_errors::Diag;
use rustc_parse::parser::Recovery;
use rustc_session::parse::ParseSess;
use rustc_span::{DUMMY_SP, FileName, Span};

pub fn parse_expr(psess: &ParseSess, source_code: &str) -> Option<P<Expr>> {
let parser = rustc_parse::unwrap_or_emit_fatal(rustc_parse::new_parser_from_source_str(
psess,
FileName::anon_source_code(source_code),
source_code.to_owned(),
));

let mut parser = parser.recovery(Recovery::Forbidden);
let mut expr = parser.parse_expr().map_err(Diag::cancel).ok()?;
if parser.token != token::Eof {
return None;
}

Normalize.visit_expr(&mut expr);
Some(expr)
}

// Erase Span information that could distinguish between identical expressions
// parsed from different source strings.
struct Normalize;

impl MutVisitor for Normalize {
const VISIT_TOKENS: bool = true;

fn visit_id(&mut self, id: &mut NodeId) {
*id = DUMMY_NODE_ID;
}

fn visit_span(&mut self, span: &mut Span) {
*span = DUMMY_SP;
}
}
18 changes: 4 additions & 14 deletions tests/ui-fulldeps/pprust-expr-roundtrip.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//@ run-pass
//@ ignore-cross-compile
//@ aux-crate: parser=parser.rs
//@ edition: 2021

// The general idea of this test is to enumerate all "interesting" expressions and check that
// `parse(print(e)) == e` for all `e`. Here's what's interesting, for the purposes of this test:
Expand All @@ -21,7 +23,6 @@

extern crate rustc_ast;
extern crate rustc_ast_pretty;
extern crate rustc_data_structures;
extern crate rustc_parse;
extern crate rustc_session;
extern crate rustc_span;
Expand All @@ -32,28 +33,17 @@ extern crate thin_vec;
#[allow(unused_extern_crates)]
extern crate rustc_driver;

use parser::parse_expr;
use rustc_ast::mut_visit::{visit_clobber, MutVisitor};
use rustc_ast::ptr::P;
use rustc_ast::*;
use rustc_ast_pretty::pprust;
use rustc_parse::{new_parser_from_source_str, unwrap_or_emit_fatal};
use rustc_session::parse::ParseSess;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::Ident;
use rustc_span::{FileName, DUMMY_SP};
use rustc_span::DUMMY_SP;
use thin_vec::{thin_vec, ThinVec};

fn parse_expr(psess: &ParseSess, src: &str) -> Option<P<Expr>> {
let src_as_string = src.to_string();

let mut p = unwrap_or_emit_fatal(new_parser_from_source_str(
psess,
FileName::Custom(src_as_string.clone()),
src_as_string,
));
p.parse_expr().map_err(|e| e.cancel()).ok()
}

// Helper functions for building exprs
fn expr(kind: ExprKind) -> P<Expr> {
P(Expr { id: DUMMY_NODE_ID, kind, span: DUMMY_SP, attrs: AttrVec::new(), tokens: None })
Expand Down
43 changes: 7 additions & 36 deletions tests/ui-fulldeps/pprust-parenthesis-insertion.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//@ run-pass
//@ ignore-cross-compile
//@ aux-crate: parser=parser.rs
//@ edition: 2021

// This test covers the AST pretty-printer's automatic insertion of parentheses
// into unparenthesized syntax trees according to precedence and various grammar
Expand Down Expand Up @@ -31,24 +33,19 @@

extern crate rustc_ast;
extern crate rustc_ast_pretty;
extern crate rustc_driver;
extern crate rustc_errors;
extern crate rustc_parse;
extern crate rustc_session;
extern crate rustc_span;

use std::mem;
use std::process::ExitCode;

use rustc_ast::ast::{DUMMY_NODE_ID, Expr, ExprKind};
use parser::parse_expr;
use rustc_ast::ast::{Expr, ExprKind};
use rustc_ast::mut_visit::{self, DummyAstNode as _, MutVisitor};
use rustc_ast::node_id::NodeId;
use rustc_ast::ptr::P;
use rustc_ast_pretty::pprust;
use rustc_errors::Diag;
use rustc_parse::parser::Recovery;
use rustc_session::parse::ParseSess;
use rustc_span::{DUMMY_SP, FileName, Span};

// Every parenthesis in the following expressions is re-inserted by the
// pretty-printer.
Expand Down Expand Up @@ -156,34 +153,6 @@ impl MutVisitor for Unparenthesize {
}
}

// Erase Span information that could distinguish between identical expressions
// parsed from different source strings.
struct Normalize;

impl MutVisitor for Normalize {
const VISIT_TOKENS: bool = true;

fn visit_id(&mut self, id: &mut NodeId) {
*id = DUMMY_NODE_ID;
}

fn visit_span(&mut self, span: &mut Span) {
*span = DUMMY_SP;
}
}

fn parse_expr(psess: &ParseSess, source_code: &str) -> Option<P<Expr>> {
let parser = rustc_parse::unwrap_or_emit_fatal(rustc_parse::new_parser_from_source_str(
psess,
FileName::anon_source_code(source_code),
source_code.to_owned(),
));

let mut expr = parser.recovery(Recovery::Forbidden).parse_expr().map_err(Diag::cancel).ok()?;
Normalize.visit_expr(&mut expr);
Some(expr)
}

fn main() -> ExitCode {
let mut status = ExitCode::SUCCESS;
let mut fail = |description: &str, before: &str, after: &str| {
Expand All @@ -199,7 +168,9 @@ fn main() -> ExitCode {
let psess = &ParseSess::new(vec![rustc_parse::DEFAULT_LOCALE_RESOURCE]);

for &source_code in EXPRS {
let expr = parse_expr(psess, source_code).unwrap();
let Some(expr) = parse_expr(psess, source_code) else {
panic!("Failed to parse original test case: {source_code}");
};

// Check for FALSE POSITIVE: pretty-printer inserting parentheses where not needed.
// Pseudocode:
Expand Down

0 comments on commit 4cb124d

Please sign in to comment.