compiletest: Refactor compile-fail to regex. #14283
Conversation
|
|
||
| let mut error_patterns = Vec::new(); | ||
| // FIXME: Would be nice to use the regex! macro here (when it's available at stage0) | ||
| let re = Regex::new(r"^.*(//)~(?P<adjusts>[\^]*)\s*(?P<kind>\S*)\s*(?P<msg>.*)").unwrap(); |
There was a problem hiding this comment.
You could wrap this in a private function that returns the regex object. Use #[cfg(stage0)] to have it construct it at runtime and unwrap, and #[cfg(not(stage0))] to have it use the macro and a static.
There was a problem hiding this comment.
This regex is unnecessarily complicated. You can ditch the ^.* at the start, remove that first group, and replace the [\^] with \^. This looks like
let re = Regex::new(r"//~(?P<adjusts>\^*)\s*(?P<kind>\S*)\s*(?P<msg>.*)").unwrap();You can tell where this starts using pos(0).
There was a problem hiding this comment.
You could wrap this in a private function that returns the regex object. Use #[cfg(stage0)] to have it construct it at runtime and unwrap, and #[cfg(not(stage0))] to have it use the macro and a static.
It's not really a perf problem, just a nice to have, think it's worth adding this (minor) complexity?
This regex is unnecessarily complicated.
Yeah, I didn't really clean up the final version, mybad.
There was a problem hiding this comment.
It's not really a perf problem, just a nice to have, think it's worth adding this (minor) complexity?
I guess the answer depends on whether we can expect to get rid of the stage0 case.
Although I actually question whether we even need it. make check-stage0-cfail tells me
make: *** No rule to make target `x86_64-apple-darwin/stage0/bin/compiletest', needed by `tmp/check-stage0-T-x86_64-apple-darwin-H-x86_64-apple-darwin-cfail.ok'. Stop
It appears the first compiletest I can build is the stage1 compiletest. It's built using the stage0 rustc, but I don't see why we can't ask the build system to compile the regex_macros crate.
That said, since it's probably more complicated than a 1-line change, it may make more sense just to stick with the dynamic approach. Of course this approach is unnecessarily recompiling the regex for every file, but I don't know if that performance hit is noticeable.
Ryman
changed the title
|
@kballard r? |
| @@ -33,6 +33,8 @@ use getopts::{optopt, optflag, reqopt}; | |||
| use common::Config; | |||
| use common::{Pretty, DebugInfoGdb, Codegen}; | |||
| use util::logv; | |||
| use regex::Regex; | |||
| use errors::EXPECTED_PATTERN; | |||
There was a problem hiding this comment.
This import is unnecessary, you're referring to the pattern as errors::EXPECTED_PATTERN below (which works because errors is in scope).
…llard Edit: This now only covers refactoring to regex.
Load proc-macros for rustc_private crates If the client support our server status notification there is no need to show the pop up for workspace fetching failures since that's already going to be shown in the status. cc rust-lang/rust-analyzer#14193
Edit: This now only covers refactoring to regex.