Skip to content

Commit 75b67c2

Browse files
committed
Fix symbol ordering for confusable idents detection.
Confusable idents detection uses a type `BTreeMap<Symbol, Span>`. This is highly dubious given that `Symbol` doesn't guarantee a meaningful order. (In practice, it currently gives an order that mostly matches source code order.) As a result, changes in `Symbol` representation make the `lint-confusable-idents.rs` test fail, because this error message: > identifier pair considered confusable between `s` and `s` is changed to this: > identifier pair considered confusable between `s` and `s` and the corresponding span pointers get swapped erroneously, leading to an incorrect "previous identifier" label. This commit sorts the relevant symbols by span before doing the checking, which ensures that the ident that appears first in the code will be mentioned first in the message. The commit also extends the test slightly to be more thorough.
1 parent 39e593a commit 75b67c2

File tree

4 files changed

+20
-4
lines changed

4 files changed

+20
-4
lines changed

src/librustc_lint/non_ascii_idents.rs

+6
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ impl EarlyLintPass for NonAsciiIdents {
5858

5959
let mut has_non_ascii_idents = false;
6060
let symbols = cx.sess.parse_sess.symbol_gallery.symbols.lock();
61+
62+
// Sort by `Span` so that error messages make sense with respect to the
63+
// order of identifier locations in the code.
64+
let mut symbols: Vec<_> = symbols.iter().collect();
65+
symbols.sort_by_key(|k| k.1);
66+
6167
for (symbol, &sp) in symbols.iter() {
6268
let symbol_str = symbol.as_str();
6369
if symbol_str.is_ascii() {

src/librustc_session/parse.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use rustc_span::hygiene::ExpnId;
1313
use rustc_span::source_map::{FilePathMapping, SourceMap};
1414
use rustc_span::{MultiSpan, Span, Symbol};
1515

16-
use std::collections::BTreeMap;
1716
use std::path::PathBuf;
1817
use std::str;
1918

@@ -64,7 +63,7 @@ impl GatedSpans {
6463
#[derive(Default)]
6564
pub struct SymbolGallery {
6665
/// All symbols occurred and their first occurrence span.
67-
pub symbols: Lock<BTreeMap<Symbol, Span>>,
66+
pub symbols: Lock<FxHashMap<Symbol, Span>>,
6867
}
6968

7069
impl SymbolGallery {

src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.rs

+2
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
#![allow(uncommon_codepoints, non_upper_case_globals)]
44

55
const: usize = 42;
6+
const s_s: usize = 42;
67

78
fn main() {
89
let s = "rust"; //~ ERROR identifier pair considered confusable
10+
let s_s = "rust2"; //~ ERROR identifier pair considered confusable
911
not_affected();
1012
}
1113

src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.stderr

+11-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: identifier pair considered confusable between `s` and `s`
2-
--> $DIR/lint-confusable-idents.rs:8:9
2+
--> $DIR/lint-confusable-idents.rs:9:9
33
|
44
LL | const s: usize = 42;
55
| -- this is where the previous identifier occurred
@@ -13,5 +13,14 @@ note: the lint level is defined here
1313
LL | #![deny(confusable_idents)]
1414
| ^^^^^^^^^^^^^^^^^
1515

16-
error: aborting due to previous error
16+
error: identifier pair considered confusable between `s_s` and `s_s`
17+
--> $DIR/lint-confusable-idents.rs:10:9
18+
|
19+
LL | const s_s: usize = 42;
20+
| --- this is where the previous identifier occurred
21+
...
22+
LL | let s_s = "rust2";
23+
| ^^^^^
24+
25+
error: aborting due to 2 previous errors
1726

0 commit comments

Comments
 (0)