Add initial support for extra_files to cttests #603
Conversation
|
So just in the spirit of trying to brainstorm alternate ideas, if we had some sort of API for running the |
lrlex/src/lib/ctbuilder.rs
Outdated
| usize: num_traits::AsPrimitive<LexerTypesT::StorageT>, | ||
| { | ||
| lrpar_config: Option<Box<dyn Fn(CTParserBuilder<LexerTypesT>) -> CTParserBuilder<LexerTypesT>>>, | ||
| lrpar_config: Option<Box<dyn Fn(CTParserBuilder<LexerTypesT>) -> CTParserBuilder<LexerTypesT> + 'a>>, |
There was a problem hiding this comment.
If we're willing to change the lifetime of this closure from being 'static to another lifetime like 'a here,
we can entirely get rid of the leak variables hacks.
This is here because Box is defaulting to 'static unless otherwise specified.
With it specified to something like 'a, the closure no longer needs to outlive the builder and main.
With that we can get rid of the leak hacks in this branch (at least, haven't looked into further cleanups yet)
Edit: It also allowed removal of the rereading and parsing of the .test yaml file.
There was a problem hiding this comment.
Going from 'static -> 'a seems fully backwards compatible (i.e. it's more general) to me, so I don't see a problem with this!
|
Please squash (if you wish). |
ratmice
force-pushed
the
cttest_extra_files
branch
from
5bf06cd to
19e3ce6
Compare
|
Squashed, I moved the lifetime changes out to their own commit before the rest, so the changes to the library and cttests are somewhat separate from one another. |
|
Thanks! |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Because `%grmtools{test_files}` needs *both* the lexer and parser
to work. We migrate this code from using the separate `CTLexerBuilder`
and `CTParserBuilder` with `rule_ids_map` to the combined method using
`lrpar_config`.
That causes `%grmtools{test_files}` to be invoked, also add an `extra_files`
keys to the `.test` file yaml. These emit the files to `$OUT_DIR` where the
`test_files` globs are relative to.
ratmice
force-pushed
the
cttest_extra_files
branch
from
19e3ce6 to
0028126
Compare
|
Had to rustfmt the first commit, should be fixed. |
2 participants
To say this isn't very pretty is a bit of an understatement, but this is my first successful attempt at implementing
extra_filesin the .yaml test output.We want to use the
CTLexerBuilder::lrpar_configcallback which implements%grmtools{test_files}behavior.In order to do that though, we must fix/hack up the lifetimes of the
cgen_helper::run_test_filesWe hack up those lifetimes in two ways:
Pathsent torun_test_files.testfile yaml from within the closure.The main issue being the borrow checker can't tell that the
Pathvariable isn't used after therun_test_filesfunction exits.I spent quite a lot of time fighting this, and this is the only thing I could manage to get working.
I manually tested that when you change one of the
.calc_inputfiles to include any kind of malformed input that it is caught correctly.