From 9b2b8a5afa833795b66267684615497c9547878d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 17 May 2020 15:51:01 -0400 Subject: [PATCH 1/3] Break tokens before checking if they are 'probably equal' Fixes #68489 When checking two `TokenStreams` to see if they are 'probably equal', we ignore the `IsJoint` information associated with each `TokenTree`. However, the `IsJoint` information determines whether adjacent tokens will be 'glued' (if possible) when construction the `TokenStream` - e.g. `[Gt Gt]` can be 'glued' to `BinOp(Shr)`. Since we are ignoring the `IsJoint` information, 'glued' and 'unglued' tokens are equivalent for determining if two `TokenStreams` are 'probably equal'. Therefore, we need to 'unglue' all tokens in the stream to avoid false negatives (which cause us to throw out the cached tokens, losing span information). --- src/librustc_ast/tokenstream.rs | 34 +++++++++++++++++-- src/test/ui/proc-macro/turbo-proc-macro.rs | 9 +++++ .../ui/proc-macro/turbo-proc-macro.stderr | 9 +++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/proc-macro/turbo-proc-macro.rs create mode 100644 src/test/ui/proc-macro/turbo-proc-macro.stderr diff --git a/src/librustc_ast/tokenstream.rs b/src/librustc_ast/tokenstream.rs index 916a5ff6f46f4..38483360c0664 100644 --- a/src/librustc_ast/tokenstream.rs +++ b/src/librustc_ast/tokenstream.rs @@ -338,8 +338,38 @@ impl TokenStream { true } - let mut t1 = self.trees().filter(semantic_tree); - let mut t2 = other.trees().filter(semantic_tree); + // When comparing two `TokenStream`s, we ignore the `IsJoint` information. + // + // However, `rustc_parse::lexer::tokentrees::TokenStreamBuilder` will + // use `Token.glue` on adjacent tokens with the proper `IsJoint`. + // Since we are ignoreing `IsJoint`, a 'glued' token (e.g. `BinOp(Shr)`) + // and its 'split'/'unglued' compoenents (e.g. `Gt, Gt`) are equivalent + // when determining if two `TokenStream`s are 'probably equal'. + // + // Therefore, we use `break_two_token_op` to convert all tokens + // to the 'unglued' form (if it exists). This ensures that two + // `TokenStream`s which differ only in how their tokens are glued + // will be considered 'probably equal', which allows us to keep spans. + // + // This is important when the original `TokenStream` contained + // extra spaces (e.g. `f :: < Vec < _ > > ( ) ;'). These extra spaces + // will be omitted when we pretty-print, which can cause the original + // and reparsed `TokenStream`s to differ in the assignment of `IsJoint`, + // leading to some tokens being 'glued' together in one stream but not + // the other. See #68489 for more details. + fn break_tokens(tree: TokenTree) -> impl Iterator { + if let TokenTree::Token(token) = &tree { + if let Some((first, second)) = token.kind.break_two_token_op() { + return SmallVec::from_buf([TokenTree::Token(Token::new(first, DUMMY_SP)), TokenTree::Token(Token::new(second, DUMMY_SP))]).into_iter() + } + } + let mut vec = SmallVec::<[_; 2]>::new(); + vec.push(tree); + vec.into_iter() + } + + let mut t1 = self.trees().filter(semantic_tree).flat_map(break_tokens); + let mut t2 = other.trees().filter(semantic_tree).flat_map(break_tokens); for (t1, t2) in t1.by_ref().zip(t2.by_ref()) { if !t1.probably_equal_for_proc_macro(&t2) { return false; diff --git a/src/test/ui/proc-macro/turbo-proc-macro.rs b/src/test/ui/proc-macro/turbo-proc-macro.rs new file mode 100644 index 0000000000000..a255955f38da0 --- /dev/null +++ b/src/test/ui/proc-macro/turbo-proc-macro.rs @@ -0,0 +1,9 @@ +// aux-build:test-macros.rs + +extern crate test_macros; + +#[test_macros::recollect_attr] +fn repro() { + f :: < Vec < _ > > ( ) ; //~ ERROR cannot find +} +fn main() {} diff --git a/src/test/ui/proc-macro/turbo-proc-macro.stderr b/src/test/ui/proc-macro/turbo-proc-macro.stderr new file mode 100644 index 0000000000000..85c93b9345c37 --- /dev/null +++ b/src/test/ui/proc-macro/turbo-proc-macro.stderr @@ -0,0 +1,9 @@ +error[E0425]: cannot find function `f` in this scope + --> $DIR/turbo-proc-macro.rs:7:5 + | +LL | f :: < Vec < _ > > ( ) ; + | ^ not found in this scope + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0425`. From 4a8ccdcc0b518e3c1878ce0be888fd85521b2026 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 19 May 2020 19:45:58 -0400 Subject: [PATCH 2/3] Use a fixed-point iteration when breaking tokens Some tokens need to be broken in a loop until we reach 'unbreakable' tokens. --- src/librustc_ast/tokenstream.rs | 45 ++++++++++++++++--- src/test/ui/proc-macro/break-token-spans.rs | 16 +++++++ .../ui/proc-macro/break-token-spans.stderr | 21 +++++++++ src/test/ui/proc-macro/turbo-proc-macro.rs | 9 ---- .../ui/proc-macro/turbo-proc-macro.stderr | 9 ---- 5 files changed, 77 insertions(+), 23 deletions(-) create mode 100644 src/test/ui/proc-macro/break-token-spans.rs create mode 100644 src/test/ui/proc-macro/break-token-spans.stderr delete mode 100644 src/test/ui/proc-macro/turbo-proc-macro.rs delete mode 100644 src/test/ui/proc-macro/turbo-proc-macro.stderr diff --git a/src/librustc_ast/tokenstream.rs b/src/librustc_ast/tokenstream.rs index 38483360c0664..075aaa7e5bc01 100644 --- a/src/librustc_ast/tokenstream.rs +++ b/src/librustc_ast/tokenstream.rs @@ -21,6 +21,8 @@ use rustc_macros::HashStable_Generic; use rustc_span::{Span, DUMMY_SP}; use smallvec::{smallvec, SmallVec}; +use log::debug; + use std::{iter, mem}; /// When the main rust parser encounters a syntax-extension invocation, it @@ -358,14 +360,47 @@ impl TokenStream { // leading to some tokens being 'glued' together in one stream but not // the other. See #68489 for more details. fn break_tokens(tree: TokenTree) -> impl Iterator { + // In almost all cases, we should have either zero or one levels + // of 'unglueing'. However, in some unusual cases, we may need + // to iterate breaking tokens mutliple times. For example: + // '[BinOpEq(Shr)] => [Gt, Ge] -> [Gt, Gt, Eq]' + let mut token_trees: SmallVec<[_; 2]>; if let TokenTree::Token(token) = &tree { - if let Some((first, second)) = token.kind.break_two_token_op() { - return SmallVec::from_buf([TokenTree::Token(Token::new(first, DUMMY_SP)), TokenTree::Token(Token::new(second, DUMMY_SP))]).into_iter() + let mut out = SmallVec::<[_; 2]>::new(); + out.push(token.clone()); + // Iterate to fixpoint: + // * We start off with 'out' containing our initial token, and `temp` empty + // * If we are able to break any tokens in `out`, then `out` will have + // at least one more element than 'temp', so we will try to break tokens + // again. + // * If we cannot break any tokens in 'out', we are done + loop { + let mut temp = SmallVec::<[_; 2]>::new(); + let mut changed = false; + + for token in out.into_iter() { + if let Some((first, second)) = token.kind.break_two_token_op() { + temp.push(Token::new(first, DUMMY_SP)); + temp.push(Token::new(second, DUMMY_SP)); + changed = true; + } else { + temp.push(token); + } + } + out = temp; + if !changed { + break; + } + } + token_trees = out.into_iter().map(|t| TokenTree::Token(t)).collect(); + if token_trees.len() != 1 { + debug!("break_tokens: broke {:?} to {:?}", tree, token_trees); } + } else { + token_trees = SmallVec::new(); + token_trees.push(tree); } - let mut vec = SmallVec::<[_; 2]>::new(); - vec.push(tree); - vec.into_iter() + token_trees.into_iter() } let mut t1 = self.trees().filter(semantic_tree).flat_map(break_tokens); diff --git a/src/test/ui/proc-macro/break-token-spans.rs b/src/test/ui/proc-macro/break-token-spans.rs new file mode 100644 index 0000000000000..ce8b9ebb4f9d2 --- /dev/null +++ b/src/test/ui/proc-macro/break-token-spans.rs @@ -0,0 +1,16 @@ +// aux-build:test-macros.rs +// Regression test for issues #68489 and #70987 +// Tests that we properly break tokens in `probably_equal_for_proc_macro` +// See #72306 +// +// Note that the weird spacing in this example is critical +// for testing the issue. + +extern crate test_macros; + +#[test_macros::recollect_attr] +fn repro() { + f :: < Vec < _ > > ( ) ; //~ ERROR cannot find + let a: Option>= true; //~ ERROR mismatched +} +fn main() {} diff --git a/src/test/ui/proc-macro/break-token-spans.stderr b/src/test/ui/proc-macro/break-token-spans.stderr new file mode 100644 index 0000000000000..caca973f252f7 --- /dev/null +++ b/src/test/ui/proc-macro/break-token-spans.stderr @@ -0,0 +1,21 @@ +error[E0425]: cannot find function `f` in this scope + --> $DIR/break-token-spans.rs:13:5 + | +LL | f :: < Vec < _ > > ( ) ; + | ^ not found in this scope + +error[E0308]: mismatched types + --> $DIR/break-token-spans.rs:14:32 + | +LL | let a: Option>= true; + | ------------------ ^^^^ expected enum `std::option::Option`, found `bool` + | | + | expected due to this + | + = note: expected enum `std::option::Option>` + found type `bool` + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0308, E0425. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/proc-macro/turbo-proc-macro.rs b/src/test/ui/proc-macro/turbo-proc-macro.rs deleted file mode 100644 index a255955f38da0..0000000000000 --- a/src/test/ui/proc-macro/turbo-proc-macro.rs +++ /dev/null @@ -1,9 +0,0 @@ -// aux-build:test-macros.rs - -extern crate test_macros; - -#[test_macros::recollect_attr] -fn repro() { - f :: < Vec < _ > > ( ) ; //~ ERROR cannot find -} -fn main() {} diff --git a/src/test/ui/proc-macro/turbo-proc-macro.stderr b/src/test/ui/proc-macro/turbo-proc-macro.stderr deleted file mode 100644 index 85c93b9345c37..0000000000000 --- a/src/test/ui/proc-macro/turbo-proc-macro.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0425]: cannot find function `f` in this scope - --> $DIR/turbo-proc-macro.rs:7:5 - | -LL | f :: < Vec < _ > > ( ) ; - | ^ not found in this scope - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0425`. From 633293fc3a54247f4308507632040424356a9e19 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 20 May 2020 15:33:58 -0400 Subject: [PATCH 3/3] Fix tests --- src/test/ui/proc-macro/break-token-spans.rs | 2 +- src/test/ui/suggestions/issue-61963.rs | 1 + src/test/ui/suggestions/issue-61963.stderr | 10 ++++++++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/test/ui/proc-macro/break-token-spans.rs b/src/test/ui/proc-macro/break-token-spans.rs index ce8b9ebb4f9d2..59dc3b5043cd7 100644 --- a/src/test/ui/proc-macro/break-token-spans.rs +++ b/src/test/ui/proc-macro/break-token-spans.rs @@ -2,7 +2,7 @@ // Regression test for issues #68489 and #70987 // Tests that we properly break tokens in `probably_equal_for_proc_macro` // See #72306 -// +// // Note that the weird spacing in this example is critical // for testing the issue. diff --git a/src/test/ui/suggestions/issue-61963.rs b/src/test/ui/suggestions/issue-61963.rs index c9d738f5a283e..666fc965f02f5 100644 --- a/src/test/ui/suggestions/issue-61963.rs +++ b/src/test/ui/suggestions/issue-61963.rs @@ -16,6 +16,7 @@ pub struct Qux(T); #[dom_struct] pub struct Foo { + //~^ ERROR trait objects without an explicit `dyn` are deprecated [bare_trait_objects] qux: Qux>, bar: Box, //~^ ERROR trait objects without an explicit `dyn` are deprecated [bare_trait_objects] diff --git a/src/test/ui/suggestions/issue-61963.stderr b/src/test/ui/suggestions/issue-61963.stderr index 0e2eb7616cf9d..62ae5fa3fe54f 100644 --- a/src/test/ui/suggestions/issue-61963.stderr +++ b/src/test/ui/suggestions/issue-61963.stderr @@ -1,5 +1,5 @@ error: trait objects without an explicit `dyn` are deprecated - --> $DIR/issue-61963.rs:20:14 + --> $DIR/issue-61963.rs:21:14 | LL | bar: Box, | ^^^ help: use `dyn`: `dyn Bar` @@ -10,5 +10,11 @@ note: the lint level is defined here LL | #![deny(bare_trait_objects)] | ^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: trait objects without an explicit `dyn` are deprecated + --> $DIR/issue-61963.rs:18:1 + | +LL | pub struct Foo { + | ^^^ help: use `dyn`: `dyn pub` + +error: aborting due to 2 previous errors