Skip to content
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

"implements" not recognized? #14

Open
icefoxen opened this issue Apr 4, 2018 · 8 comments
Open

"implements" not recognized? #14

icefoxen opened this issue Apr 4, 2018 · 8 comments

Comments

@icefoxen
Copy link

icefoxen commented Apr 4, 2018

Hi, I'm trying to use this to make a webidl-to-Rust compiler: https://github.com/icefoxen/orbweaver/

For the Firefox 59 webidl it parses about 2/3 of the files without errors but one of the main errors is that it doesn't appear to recognize the implements keyword, like so:

firefox_webidl/ScriptProcessorNode.webidl: UnrecognizedToken { token: Some((703, Identifier("implements"), 713)), expected: ["\"includes\""] }

Not sure whether or not this is in the WebIDL standard or is some (inevitable) extension, since I can't find it in the grammar, but I also am only just getting started with this.

Thank you!

@sgodwincs
Copy link
Owner

I remember it being removed about 5-6 months ago I think. They replaced it with includes, but I'm not entirely sure the semantics are the same. I was hoping the Firefox WebIDLs would be updated in a relatively short amount of time, but it seems they haven't yet.

I can add parsing of it back, but it might be alright to just do string replacement from implements to includes for now.

@sgodwincs
Copy link
Owner

sgodwincs commented Apr 4, 2018

I just added a commit to add support for parsing it. Though, I'm going to wait to publish the new 0.6.0 version until #11 has been resolved, since it hopefully should be shortly. In the meantime, you can just use the master branch directly as a dependency.

Also, your wording seemed to imply there was more than one type of error?

@icefoxen
Copy link
Author

icefoxen commented Apr 4, 2018

Thank you! That seems to fix a large portion of the failures.

There were a few different errors, but I didn't go too deep into them, this was just that one that stood out. The full list is here: https://gist.github.com/icefoxen/465c4ecac435600feb4a30ba786330fb

This is going from the webidl's in the Firefox source code from here: https://archive.mozilla.org/pub/firefox/releases/59.0/SOURCE

The program in https://github.com/icefoxen/orbweaver/ downloads and extracts the appropriate files as part of its build.rs, so you can just run that if you want (though it does suck down all 300 mb of Firefox's source code, so it takes a while).

...oh, or you can browse them at https://dxr.mozilla.org/mozilla-central/source/dom/webidl , which I THINK is the same version... (edit: Nope, it totally isn't.)

@icefoxen
Copy link
Author

icefoxen commented Apr 4, 2018

A lot of the unrecognized token: Semicolon, Expected one of ":" or "{" seems to come from the structure interface Foo; which appears invalid; should probably be interface Foo {}. There's lots of other semicolons in strange places, unfortunately... going to try to track them down.

Unrecognized token OtherLiteral('#')surprised me but a few of firefox's webidl files have#ifdef` annotations in them. Those are easy to get rid of.

Could not parse "MediaDeviceInfo.webidl": Unrecognized token Semicolon found at 717:718 Expected one of "(", ")", ",", "...", ">", "?", "Identifier", "attribute", "callback", "const", "deleter", "dictionary", "enum", "getter", "implements", "includes", "inherit", "interface", "iterable", "maplike", "namespace", "or", "partial", "required", "setlike", "setter", "static", "stringifier", "typedef" or "unrestricted" seems to be from interface Foo {}; which appears to be invalid.

Could not parse "Console.webidl": Unrecognized token Const found at 1536:1541 Expected one of "(", "ArrayBuffer", "ByteString", "DOMString", "DataView", "Error", "Float32Array", "Float64Array", "FrozenArray", "Identifier", "Int16Array", "Int32Array", "Int8Array", "Promise", "USVString", "Uint16Array", "Uint32Array", "Uint8Array", "Uint8ClampedArray", "any", "boolean", "byte", "double", "float", "long", "object", "octet", "readonly", "record", "sequence", "short", "symbol", "unrestricted", "unsigned" or "void" comes from namespace Foo { const type BAR = ...; } which... I think has been replaced with readonly?

...and so on. So, it appears you probably implement the WebIDL spec correctly, but you are probably also the only one. 🙄 Not sure what to do about this.

@sgodwincs
Copy link
Owner

sgodwincs commented Apr 4, 2018

I haven't yet looked into in detail at the above errors, but I am curious if you have tried using the WebIDLs in servo. I know that at least these will be parsed 100% correctly as I have written tests for them in parse_test.rs (well, at least at some point when I downloaded them and saved them in the tests folder).

Unfortunately, the WebIDLs used by Firefox and Chrome sometimes just add arbitrary grammar extensions that don't seem to be specified anywhere. I do recall the interface Foo; one which seems like it's just a forward declaration. Part of the problem is that the WebIDL specification I implement is also the newer one, the older one is specified in https://www.w3.org/TR/WebIDL-1/, but I haven't actually taken the time to see what the differences exactly are in the grammar.

I'm not against adding extensions into the grammar, but I also don't want to stray too far from the specification. Maybe a lot of it can be handled directly in the lexer where we can just output the correct tokens (i.e. interface Foo; outputs tokens interface Foo { } ; instead). This would make it a little bit more complicated and less clean, but it should be doable for the errors you mentioned at first glance. The #ifdef conditional one seems to be a bit weird, should it just be completely ignored?

@icefoxen
Copy link
Author

icefoxen commented Apr 4, 2018

It does not appear that firefox's IDL conforms to the earlier spec either, annoyingly. It might be some weird mutant hybrid and it would surprise me not at all. I haven't even started looking at what Chrome or Edge do, but I expect them to be uniquely terrible in their own ways.

I think I will start with the servo spec at least to get rolling, thank you! If the API described is mostly the same that will be enough for a proof of concept.

@sgodwincs
Copy link
Owner

sgodwincs commented Apr 4, 2018

I went through all of the errors you listed, they all seem to be one of the following:

  1. Lines of the form interface Foo;
  2. Using jsonifier; as an interface member. I think this is meant to be serializer which is supported by the spec. In PaymentResponse.webidl, it has
  // TODO: Use serializer once available. (Bug 863402)
  // serializer = {attribute};
  jsonifier;
  1. Using const as a namespace member. I agree that this is probably close enough to readonly.
  2. Using the keyword includes as the name of an operation (in IDBKeyRange.webidl).
  3. #ifdef and #endif conditionals
  4. Use of legacycaller special operation.

Number 6 is similar to the implements situation, it was a keyword removed from the spec months ago. This should be supported for backwards compatibility. Number 4 should be OK to handle since keywords are allowed to be used as identifiers as long as they are prefixed by _ (i.e. _includes will end up being an identifier with value includes). Everything else should be doable, but I'm not sure what to do with the actual conditional #ifdef. It seems the only practical option is to just ignore it, but should everything else inside be ignored?

I probably will take the option of just having separate preprocessing functions for different WebIDL "variants" as to avoid polluting the parser with strange workarounds.

@icefoxen
Copy link
Author

icefoxen commented Apr 4, 2018

That sounds fairly reasonable; you obviously know more about the topic than I do, who just cracked open the spec for the first time yesterday. I was vaguely considering writing a patch for Firefox, but I think I'm going to have to just leave it for now, since this is still just an experiment. Implementing those transformations as a preprocessor or such seems relatively doable.

For #ifdef's I was going to just remove them and anything in the if section, since they seem to be quite uncommon -- there's five of them in all, along with one sad lonely #if defined(...) for some reason. There's no logical operations, #ifndef, or anything like that to gum things up too much, so again, for the purposes of my experiment it's not a big deal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants