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

Add witx crate, to parse and validate witx #90

Merged
merged 7 commits into from
Sep 13, 2019

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Sep 13, 2019

As a follow on to #74, I'm submitting the witx crate, adapted from the Lucet project (https://github.com/fastly/lucet).

witx parses and validates a witx file into the representation given in tools/witx/src/ast.rs. The validation rules are written in src/validation.rs. The crate produces a binary, witx, which validates a witx file, or reports an error if it fails to validate.

witx uses stable rust, and has only two crate dependencies - failure, a very-nearly-standard error reporting library, and clap, a popular argument parsing library. (clap and the executable could be separated out into a separate crate if necessary to drop that dependency).

This PR also fixes one minor syntax error in the existing witx files. It is already useful!

After this is merged, I will make a PR to add Azure Pipelines integration to ensure that this code builds without errors, and validates all checked-in witx files, as a condition to merge each PR.

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess its time I learnt this newfangled language.

@@ -0,0 +1,236 @@
pub use crate::lexer::LexError;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put a copyright header at the top of each source:

/*                                                                               
 * Copyright 2019 WebAssembly Community Group participants                       
 *                                                                               
 * Licensed under the Apache License, Version 2.0 (the "License");               
 * you may not use this file except in compliance with the License.              
 * You may obtain a copy of the License at                                       
 *                                                                               
 *     http://www.apache.org/licenses/LICENSE-2.0                                
 *                                                                               
 * Unless required by applicable law or agreed to in writing, software           
 * distributed under the License is distributed on an "AS IS" BASIS,             
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.      
 * See the License for the specific language governing permissions and           
 * limitations under the License.                                                
 */                                                                              

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe a LICENCE at the top level? (or maybe the top level of the entire repo?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec interpreter doesn't have copyright notices in the source files, and I think it's a reasonable example to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a LICENSE file with that exact text, fixed the license field in cargo toml to leave out the llvm-exception part

@pchickey
Copy link
Contributor Author

Per @sunfishcode's suggestion, I renamed the crate to witx, since it will be essentially the standard witx implementation for Rust.

@pchickey pchickey changed the title Add witx-frontend crate, to parse and validate witx Add witx crate, to parse and validate witx Sep 13, 2019
@pchickey pchickey force-pushed the pch/add_witx_frontend branch from 369d69f to cf799b4 Compare September 13, 2019 02:20
@pchickey
Copy link
Contributor Author

I also published this version to crates.io to reserve the package name, and added Dan as an owner.

@sunfishcode
Copy link
Member

Looks good! Since we discussed this in the last meeting, let's go ahead and merge, and we can continue to iterate with PRs and issues from here.

@sunfishcode sunfishcode merged commit 5d10b2c into master Sep 13, 2019
sunfishcode added a commit to sunfishcode/WASI that referenced this pull request Sep 13, 2019
@pchickey pchickey deleted the pch/add_witx_frontend branch September 14, 2019 17:26
sunfishcode added a commit that referenced this pull request Sep 18, 2019
* Create a structure for phased development.

This PR takes the `witx` concept from #74 and creates a directory
structure around it for enabling phased development of new APIs
and changes to existing APIs.

See the README.md changes for details.

* Apply the validation fix from #90 to the new copies.
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

Successfully merging this pull request may close these issues.

3 participants