Skip to content

Commit 525ed4c

Browse files
committed
Read the whole test file before parsing directives
Few tests are larger than a handful of kilobytes, and nowadays we scan the whole file for directives anyway, so there's little reason not to just read the whole thing up-front. This avoids having to deal with I/O within `iter_directives`, which should make it easier to overhaul directive processing.
1 parent ff5be13 commit 525ed4c

3 files changed

Lines changed: 32 additions & 46 deletions

File tree

‎src/tools/compiletest/src/directives.rs‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
use std::collections::HashSet;
2-
use std::env;
3-
use std::fs::File;
4-
use std::io::BufReader;
5-
use std::io::prelude::*;
62
use std::process::Command;
3+
use std::{env, fs};
74

85
use camino::{Utf8Path,Utf8PathBuf};
96
use semver::Version;
@@ -54,18 +51,19 @@ pub struct EarlyProps {
5451

5552
implEarlyProps{
5653
pubfnfrom_file(config:&Config,testfile:&Utf8Path) -> Self{
57-
let file = File::open(testfile.as_std_path()).expect("open test file to parse earlyprops");
58-
Self::from_reader(config, testfile, file)
54+
let file_contents =
55+
fs::read_to_string(testfile).expect("read test file to parse earlyprops");
56+
Self::from_file_contents(config, testfile,&file_contents)
5957
}
6058

61-
pubfnfrom_reader<R:Read>(config:&Config,testfile:&Utf8Path,rdr:R) -> Self{
59+
pubfnfrom_file_contents(config:&Config,testfile:&Utf8Path,file_contents:&str) -> Self{
6260
letmut props = EarlyProps::default();
6361
letmut poisoned = false;
6462
iter_directives(
6563
config.mode,
6664
&mut poisoned,
6765
testfile,
68-
rdr,
66+
file_contents,
6967
// (dummy comment to force args into vertical layout)
7068
&mut |ref ln:DirectiveLine<'_>| {
7169
parse_and_update_aux(config, ln, testfile,&mut props.aux);
@@ -362,15 +360,15 @@ impl TestProps {
362360
fnload_from(&mutself,testfile:&Utf8Path,test_revision:Option<&str>,config:&Config){
363361
letmut has_edition = false;
364362
if !testfile.is_dir(){
365-
letfile = File::open(testfile.as_std_path()).unwrap();
363+
letfile_contents = fs::read_to_string(testfile).unwrap();
366364

367365
letmut poisoned = false;
368366

369367
iter_directives(
370368
config.mode,
371369
&mut poisoned,
372370
testfile,
373-
file,
371+
&file_contents,
374372
&mut |ref ln:DirectiveLine<'_>| {
375373
if !ln.applies_to_test_revision(test_revision){
376374
return;
@@ -859,7 +857,7 @@ fn iter_directives(
859857
mode:TestMode,
860858
poisoned:&mutbool,
861859
testfile:&Utf8Path,
862-
rdr:implRead,
860+
file_contents:&str,
863861
it:&mutdynFnMut(DirectiveLine<'_>),
864862
){
865863
if testfile.is_dir(){
@@ -886,16 +884,7 @@ fn iter_directives(
886884
}
887885
}
888886

889-
letmut rdr = BufReader::with_capacity(1024, rdr);
890-
letmut ln = String::new();
891-
letmut line_number = 0;
892-
893-
loop{
894-
line_number += 1;
895-
ln.clear();
896-
if rdr.read_line(&mut ln).unwrap() == 0{
897-
break;
898-
}
887+
for(line_number, ln)in(1..).zip(file_contents.lines()){
899888
let ln = ln.trim();
900889

901890
letSome(directive_line) = line_directive(line_number, ln)else{
@@ -1359,13 +1348,13 @@ where
13591348
Some((min, max))
13601349
}
13611350

1362-
pub(crate)fnmake_test_description<R:Read>(
1351+
pub(crate)fnmake_test_description(
13631352
config:&Config,
13641353
cache:&DirectivesCache,
13651354
name:String,
13661355
path:&Utf8Path,
13671356
filterable_path:&Utf8Path,
1368-
src:R,
1357+
file_contents:&str,
13691358
test_revision:Option<&str>,
13701359
poisoned:&mutbool,
13711360
) -> CollectedTestDesc{
@@ -1380,7 +1369,7 @@ pub(crate) fn make_test_description<R: Read>(
13801369
config.mode,
13811370
&mut local_poisoned,
13821371
path,
1383-
src,
1372+
file_contents,
13841373
&mut |ref ln @ DirectiveLine{ line_number, .. }| {
13851374
if !ln.applies_to_test_revision(test_revision){
13861375
return;

‎src/tools/compiletest/src/directives/tests.rs‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::io::Read;
2-
31
use camino::Utf8Path;
42
use semver::Version;
53

@@ -10,12 +8,12 @@ use crate::directives::{
108
};
119
usecrate::executor::{CollectedTestDesc,ShouldPanic};
1210

13-
fnmake_test_description<R:Read>(
11+
fnmake_test_description(
1412
config:&Config,
1513
name:String,
1614
path:&Utf8Path,
1715
filterable_path:&Utf8Path,
18-
src:R,
16+
file_contents:&str,
1917
revision:Option<&str>,
2018
) -> CollectedTestDesc{
2119
let cache = DirectivesCache::load(config);
@@ -26,7 +24,7 @@ fn make_test_description<R: Read>(
2624
name,
2725
path,
2826
filterable_path,
29-
src,
27+
file_contents,
3028
revision,
3129
&mut poisoned,
3230
);
@@ -226,14 +224,13 @@ fn cfg() -> ConfigBuilder {
226224
}
227225

228226
fnparse_rs(config:&Config,contents:&str) -> EarlyProps{
229-
let bytes = contents.as_bytes();
230-
EarlyProps::from_reader(config,Utf8Path::new("a.rs"), bytes)
227+
EarlyProps::from_file_contents(config,Utf8Path::new("a.rs"), contents)
231228
}
232229

233230
fncheck_ignore(config:&Config,contents:&str) -> bool{
234231
let tn = String::new();
235232
let p = Utf8Path::new("a.rs");
236-
let d = make_test_description(&config, tn, p, p,std::io::Cursor::new(contents),None);
233+
let d = make_test_description(&config, tn, p, p, contents,None);
237234
d.ignore
238235
}
239236

@@ -243,9 +240,9 @@ fn should_fail() {
243240
let tn = String::new();
244241
let p = Utf8Path::new("a.rs");
245242

246-
let d = make_test_description(&config, tn.clone(), p, p,std::io::Cursor::new(""),None);
243+
let d = make_test_description(&config, tn.clone(), p, p,"",None);
247244
assert_eq!(d.should_panic,ShouldPanic::No);
248-
let d = make_test_description(&config, tn, p, p,std::io::Cursor::new("//@ should-fail"),None);
245+
let d = make_test_description(&config, tn, p, p,"//@ should-fail",None);
249246
assert_eq!(d.should_panic,ShouldPanic::Yes);
250247
}
251248

@@ -778,9 +775,8 @@ fn threads_support() {
778775
}
779776
}
780777

781-
fnrun_path(poisoned:&mutbool,path:&Utf8Path,buf:&[u8]){
782-
let rdr = std::io::Cursor::new(&buf);
783-
iter_directives(TestMode::Ui, poisoned, path, rdr,&mut |_| {});
778+
fnrun_path(poisoned:&mutbool,path:&Utf8Path,file_contents:&str){
779+
iter_directives(TestMode::Ui, poisoned, path, file_contents,&mut |_| {});
784780
}
785781

786782
#[test]
@@ -789,7 +785,7 @@ fn test_unknown_directive_check() {
789785
run_path(
790786
&mut poisoned,
791787
Utf8Path::new("a.rs"),
792-
include_bytes!("./test-auxillary/unknown_directive.rs"),
788+
include_str!("./test-auxillary/unknown_directive.rs"),
793789
);
794790
assert!(poisoned);
795791
}
@@ -800,7 +796,7 @@ fn test_known_directive_check_no_error() {
800796
run_path(
801797
&mut poisoned,
802798
Utf8Path::new("a.rs"),
803-
include_bytes!("./test-auxillary/known_directive.rs"),
799+
include_str!("./test-auxillary/known_directive.rs"),
804800
);
805801
assert!(!poisoned);
806802
}
@@ -811,7 +807,7 @@ fn test_error_annotation_no_error() {
811807
run_path(
812808
&mut poisoned,
813809
Utf8Path::new("a.rs"),
814-
include_bytes!("./test-auxillary/error_annotation.rs"),
810+
include_str!("./test-auxillary/error_annotation.rs"),
815811
);
816812
assert!(!poisoned);
817813
}
@@ -822,29 +818,29 @@ fn test_non_rs_unknown_directive_not_checked() {
822818
run_path(
823819
&mut poisoned,
824820
Utf8Path::new("a.Makefile"),
825-
include_bytes!("./test-auxillary/not_rs.Makefile"),
821+
include_str!("./test-auxillary/not_rs.Makefile"),
826822
);
827823
assert!(!poisoned);
828824
}
829825

830826
#[test]
831827
fntest_trailing_directive(){
832828
letmut poisoned = false;
833-
run_path(&mut poisoned,Utf8Path::new("a.rs"),b"//@ only-x86 only-arm");
829+
run_path(&mut poisoned,Utf8Path::new("a.rs"),"//@ only-x86 only-arm");
834830
assert!(poisoned);
835831
}
836832

837833
#[test]
838834
fntest_trailing_directive_with_comment(){
839835
letmut poisoned = false;
840-
run_path(&mut poisoned,Utf8Path::new("a.rs"),b"//@ only-x86 only-arm with comment");
836+
run_path(&mut poisoned,Utf8Path::new("a.rs"),"//@ only-x86 only-arm with comment");
841837
assert!(poisoned);
842838
}
843839

844840
#[test]
845841
fntest_not_trailing_directive(){
846842
letmut poisoned = false;
847-
run_path(&mut poisoned,Utf8Path::new("a.rs"),b"//@ revisions: incremental");
843+
run_path(&mut poisoned,Utf8Path::new("a.rs"),"//@ revisions: incremental");
848844
assert!(!poisoned);
849845
}
850846

‎src/tools/compiletest/src/lib.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,8 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
892892
// `CollectedTest` that can be handed over to the test executor.
893893
collector.tests.extend(revisions.into_iter().map(|revision| {
894894
// Create a test name and description to hand over to the executor.
895-
let src_file = fs::File::open(&test_path).expect("open test file to parse ignores");
895+
let file_contents =
896+
fs::read_to_string(&test_path).expect("read test file to parse ignores");
896897
let(test_name, filterable_path) =
897898
make_test_name_and_filterable_path(&cx.config, testpaths, revision);
898899
// Create a description struct for the test/revision.
@@ -904,7 +905,7 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
904905
test_name,
905906
&test_path,
906907
&filterable_path,
907-
src_file,
908+
&file_contents,
908909
revision,
909910
&mut collector.poisoned,
910911
);

0 commit comments

Comments
 (0)