wasmtime / PR #1793 💖 wiggle: escape rust keywords, allow... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #1793 💖 wiggle: escape rust keywords, allow...

Wasmtime GitHub notifications bot (May 30 2020 at 06:37):

katie-martin-fastly opened PR #1793 from ktm/beep-beep-keywords-to-the-jeep to master:

# Overview

This commit makes changes to the wiggle::from_witx procedural in order
to allow for escaping strict and reserved Rust keywords.

Additionally, this commit introduces the ability to use a witx_literal
field in the {..} object provided as an argument to
wiggle::from_witx. This field allows for witx documents to be provided
as inline string literals.

Documentation comments are added to the methods of
wiggle_generate::names::Names struct responsible for generating
proc_macro2::Ident words.

## Keyword Escaping

Today, an interface that includes witx identifiers that conflict with
with Rust syntax will cause the from_witx macro to panic at
compilation time.

Here is a small example (adapted from
/crates/wiggle/tests/keywords.rs) that demonstrates this issue:
;; Attempts to define a module `self`, containing a trait `Self`. Both
;; of these are reserved keywords, and will thus cause a compilation
;; error.
(module $self
    (@interface func (export "betchya_cant_implement_this")
    )
)
Building off of code that (as of master today)
[demonstrates a strategy][esc] for escaping keywords, we introduce an
internal escaping module to generate/src/config.rs that contains
code responsible for escaping Rust keywords in a generalized manner.

[esc]: https://github.com/bytecodealliance/wasmtime/blob/0dd77d36f895df70c6e82758f23f553365c2f25f/crates/wiggle/generate/src/names.rs#L106

Some code related to special cases, such as accounting for
[errno::2big][err] while generating names for enum variants, is moved
into this module as well.

[err]: https://github.com/WebAssembly/WASI/blob/master/phases/snapshot/docs.md#-errno-enumu16

As mentioned in the document comments of this diff, we do not include
weak keywords like 'static or union. Their semantics do not impact
us in the same way from a code generation perspective.

## witx_literal

First, some background. Trait names, type names, and so on use a
camel-cased naming convention. As such, Self is the only keyword that
can potentially conflict with these identifiers. (See the [Rust
Reference][key] for a complete list of strict, reserved, and weak
keywords.)

When writing tests, this meant that many tests had to be outlined into
separate files, as items with the name $self could not be defined in
the same namespace. As such, it seemed like a worthwhile feature to
implement while the above work was being developed.

The most important function to note is the load_document inherent
method added to WitxConf, and that WitxConf is now an enum
containing either (a) a collection of paths, identical to its current
functionality, or (b) a single string literal.

Note that a witx document given to from_witx using a string literal
provided to from_witx cannot include use (..) directives, per
the witx::parse documentation.
(See: https://docs.rs/witx/0.8.5/witx/fn.parse.html)

Two newtypes, Paths and Literal, are introduced to facilitate the
parsing of WitxConf values. Their public API and trait implementations
has been kept to the minimum required to satisfy compilation in order to
limit the scope of this diff. Additional surface for external consumers
can be added in follow-up commits if deemed necessary in review.

:sparkling_heart: :sparkles:

Wasmtime GitHub notifications bot (May 30 2020 at 08:33):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (May 30 2020 at 08:33):

bjorn3 created PR Review Comment:

Is it appropriate to add a joke of this kind in the source?

Wasmtime GitHub notifications bot (May 30 2020 at 18:29):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (May 30 2020 at 18:29):

pchickey created PR Review Comment:

Yes

Wasmtime GitHub notifications bot (May 30 2020 at 18:46):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (May 30 2020 at 18:46):

pchickey submitted PR Review.

Wasmtime GitHub notifications bot (May 30 2020 at 18:46):

pchickey created PR Review Comment:

    /// function will return `None`.
    ///
    /// This functionality is a short-term fix that keeps WASI working. Instead of expanding these sort of special cases,
    /// we should replace this function by having the user provide a mapping of witx identifiers to Rust identifiers in the
    /// arguments to the macro.

Wasmtime GitHub notifications bot (May 30 2020 at 20:10):

pchickey updated PR #1793 from ktm/beep-beep-keywords-to-the-jeep to master:

# Overview

This commit makes changes to the wiggle::from_witx procedural in order
to allow for escaping strict and reserved Rust keywords.

Additionally, this commit introduces the ability to use a witx_literal
field in the {..} object provided as an argument to
wiggle::from_witx. This field allows for witx documents to be provided
as inline string literals.

Documentation comments are added to the methods of
wiggle_generate::names::Names struct responsible for generating
proc_macro2::Ident words.

## Keyword Escaping

Today, an interface that includes witx identifiers that conflict with
with Rust syntax will cause the from_witx macro to panic at
compilation time.

Here is a small example (adapted from
/crates/wiggle/tests/keywords.rs) that demonstrates this issue:
;; Attempts to define a module `self`, containing a trait `Self`. Both
;; of these are reserved keywords, and will thus cause a compilation
;; error.
(module $self
    (@interface func (export "betchya_cant_implement_this")
    )
)
Building off of code that (as of master today)
[demonstrates a strategy][esc] for escaping keywords, we introduce an
internal escaping module to generate/src/config.rs that contains
code responsible for escaping Rust keywords in a generalized manner.

[esc]: https://github.com/bytecodealliance/wasmtime/blob/0dd77d36f895df70c6e82758f23f553365c2f25f/crates/wiggle/generate/src/names.rs#L106

Some code related to special cases, such as accounting for
[errno::2big][err] while generating names for enum variants, is moved
into this module as well.

[err]: https://github.com/WebAssembly/WASI/blob/master/phases/snapshot/docs.md#-errno-enumu16

As mentioned in the document comments of this diff, we do not include
weak keywords like 'static or union. Their semantics do not impact
us in the same way from a code generation perspective.

## witx_literal

First, some background. Trait names, type names, and so on use a
camel-cased naming convention. As such, Self is the only keyword that
can potentially conflict with these identifiers. (See the [Rust
Reference][key] for a complete list of strict, reserved, and weak
keywords.)

When writing tests, this meant that many tests had to be outlined into
separate files, as items with the name $self could not be defined in
the same namespace. As such, it seemed like a worthwhile feature to
implement while the above work was being developed.

The most important function to note is the load_document inherent
method added to WitxConf, and that WitxConf is now an enum
containing either (a) a collection of paths, identical to its current
functionality, or (b) a single string literal.

Note that a witx document given to from_witx using a string literal
provided to from_witx cannot include use (..) directives, per
the witx::parse documentation.
(See: https://docs.rs/witx/0.8.5/witx/fn.parse.html)

Two newtypes, Paths and Literal, are introduced to facilitate the
parsing of WitxConf values. Their public API and trait implementations
has been kept to the minimum required to satisfy compilation in order to
limit the scope of this diff. Additional surface for external consumers
can be added in follow-up commits if deemed necessary in review.

:sparkling_heart: :sparkles:

Wasmtime GitHub notifications bot (May 30 2020 at 20:11):

pchickey merged PR #1793.

Last updated: Oct 23 2024 at 20:03 UTC