-
-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Merged by Bors] - Module parsing #2411
Conversation
Test262 conformance changes
Fixed tests (747):
New panics (12):
|
Codecov Report
@@ Coverage Diff @@
## main #2411 +/- ##
==========================================
- Coverage 49.40% 48.56% -0.85%
==========================================
Files 380 386 +6
Lines 37884 38572 +688
==========================================
+ Hits 18718 18731 +13
- Misses 19166 19841 +675
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
9bd4c8d
to
d6d956a
Compare
fe39f08
to
27c899f
Compare
5e7cfe5
to
dc54c88
Compare
Rebased. I'll start contributing to this on the next days |
Follows from #2528, and should complement #2411 to implement the module import hooks. ~~Similarly to the Intl/ICU4X PR (#2478), this has a lot of trivial changes caused by the new lifetimes. I thought about passing the queue and the hooks by value, but it was very painful having to wrap everything with `Rc` in order to be accessible by the host. In contrast, `&dyn` can be easily provided by the host and has the advantage of not requiring additional allocations, with the downside of adding two more lifetimes to our `Context`, but I think it's worth.~~ I was able to unify all lifetimes into the shortest one of the three, making our API just like before! Changes: - Added a new `HostHooks` trait and a `&dyn HostHooks` field to `Context`. This allows hosts to implement the trait for their custom type, then pass it to the context. - Added a new `JobQueue` trait and a `&dyn JobQueue` field to our `Context`, allowing custom event loops and other fun things. - Added two simple implementations of `JobQueue`: `IdleJobQueue` which does nothing and `SimpleJobQueue` which runs all jobs until all successfully complete or until any of them throws an error. - Modified `boa_cli` to run all jobs until the queue is empty, even if a job returns `Err`. This also prints all errors to the user.
Extracted from #2411 to reduce its size a bit. This PR: - Renames `Identifier` to `IdentifierName`, which is the name stated in the spec. - Renames the utility function `check_parser` to `check_script_parser` to prepare for modules. - Adds some missing `#[inline]` and rewrites some patterns.
Extracted from #2411 to reduce its size a bit. This PR: - Renames `Identifier` to `IdentifierName`, which is the name stated in the spec. - Renames the utility function `check_parser` to `check_script_parser` to prepare for modules. - Adds some missing `#[inline]` and rewrites some patterns.
dc54c88
to
df178f2
Compare
Slightly related to #2411 since we need an API to pass module files, but more useful for #1760, #1313 and other error reporting issues. It changes the following: - Introduces a new `Source` API to store the path of a provided file or `None` if the source is a plain string. - Improves the display of `boa_tester` to show the path of the tests being run. This also enables hyperlinks to directly jump to the tested file from the VS terminal. - Adjusts the repo to this change. Hopefully, this will improve our error display in the future.
Another change extracted from #2411. This PR changes the following: - Improves our identifier parsing with a new `Identifier` parser that unifies parsing for `IdentifierReference`, `BindingIdentifier` and `LabelIdentifier`. - Slightly improves some error messages. - Extracts our manual initialization of static `Sym`s with a new `static_syms` proc macro. - Adds `set_module_mode` and `module_mode` to the cursor to prepare for modules.
Another change extracted from #2411. This PR changes the following: - Improves our identifier parsing with a new `Identifier` parser that unifies parsing for `IdentifierReference`, `BindingIdentifier` and `LabelIdentifier`. - Slightly improves some error messages. - Extracts our manual initialization of static `Sym`s with a new `static_syms` proc macro. - Adds `set_module_mode` and `module_mode` to the cursor to prepare for modules.
b4fb15d
to
f0979cf
Compare
661b39f
to
d4588ef
Compare
Parsing is done! There are still some missing details like checking Some tests will obviously panic because this doesn't implement the runtime part of modules, but it would be better to do that on a follow-up PR since this one is getting big enough already. Also, I removed the link to the modules issue to avoid Bors closing it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, I'm finde with merging the panics if we can get them fixed before the next release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work on this! 😄 I'd be fine with merging this as is as well, but fixing the panics should definitely be focused.
bors r+ |
I'm creating this draft PR, since I wanted to have some early feedback, and because I though I would have time to finish it last week, but I got caught up with other stuff. Feel free to contribute :) The main thing here is that I have divided `eval()`, `parse()` and similar functions so that they can decide if they are parsing scripts or modules. Let me know your thoughts. Then, I was checking the import & export parsing, and I noticed we are using `TokenKind::Identifier` for `IdentifierName`, so I changed that name. An `Identifier` is an `IdentifierName` that isn't a `ReservedWord`. This means we should probably also adapt all `IdentifierReference`, `BindingIdentifier` and so on parsing. I already created an `Identifier` parser. Something interesting there is that `await` is not a valid `Identifier` if the goal symbol is `Module`, as you can see in the [spec](https://tc39.es/ecma262/#prod-LabelIdentifier), but currently we don't have that information in the `InputElement` enumeration, we only have `Div`, `RegExp` and `TemplateTail`. How could we approach this? Co-authored-by: jedel1043 <[email protected]>
Pull request successfully merged into main. Build succeeded: |
I'm creating this draft PR, since I wanted to have some early feedback, and because I though I would have time to finish it last week, but I got caught up with other stuff. Feel free to contribute :)
The main thing here is that I have divided
eval()
,parse()
and similar functions so that they can decide if they are parsing scripts or modules. Let me know your thoughts.Then, I was checking the import & export parsing, and I noticed we are using
TokenKind::Identifier
forIdentifierName
, so I changed that name. AnIdentifier
is anIdentifierName
that isn't aReservedWord
. This means we should probably also adapt allIdentifierReference
,BindingIdentifier
and so on parsing. I already created anIdentifier
parser.Something interesting there is that
await
is not a validIdentifier
if the goal symbol isModule
, as you can see in the spec, but currently we don't have that information in theInputElement
enumeration, we only haveDiv
,RegExp
andTemplateTail
. How could we approach this?