[naga] Comments parsing (naga + wgsl)

[Vrixyz]

Connections
None as far as I know.

Description
Generating automated documentation à la https://docs.rs is very useful, but currently difficult.

naga lacks information about comments to be too helpful, so this pull requests adds support to parsing those.

Testing

• wgmath documentation uses Support comments when generating documentation jannik4/shader_docs#1 to add documentation strings to it, it's based on an older version of this PR, based on wgpu 0.22 branch.
• A cargo xtask test run doesn't yield more errors than trunk.

Status

• Add a token DocComment
• Add a token DocCommentModule
• Rust syntax was chosen: [naga] Comments parsing (naga + wgsl) #6364 (comment)
  • Parse /// comments
  • Parse /** */ comments
  • Parse //! module comments
  • Parse /*! */ module comments
• Add comments to ast::Struct
• Support comments for multiple items
  • struct
    • struct members
  • var (global variables)
  • consts
  • functions
  • module level comment
    • For maximum compatibility with naga_oil, this is implemented with its own syntax: they should start with //!.
      • because naga_oil injects the imported modules at the beginning of the file, and not necessarily where the import line was written exactly. That prompted that commit: 02caf3a
  • alias ; 🚫 I consider this out of scope for this PR.
• Add comments list in naga::Module
• Added more test and adapted existing tests

Checklist

• Run cargo fmt.
• Run cargo clippy. If applicable, add:
  • --target wasm32-unknown-unknown
  • --target wasm32-unknown-emscripten
• Run cargo xtask test to run tests.
• Add change to CHANGELOG.md. See simple instructions inside file.

[jimblandy]

This doesn't get the comments out of the front end, though, does it?

It seems to me it could be fine to just add a field like:

    doc_comments: Option<Box<DocComments>>,

to naga::Module, and then have something like

struct DocComments {
    types: FastIndexMap<Handle<Type>, Vec<Span>>,
    global_variables: FastIndexMap<Handle<GlobalVariable>, Vec<Span>>,
    ...
}

I think it would make sense for Naga to adopt Rust's /// syntax as a WGSL extension.

[Vrixyz]

This doesn't get the comments out of the front end, though, does it?

Correct, I'm looking into propagating them to the module now ; my first intuition was to add comments to naga::TypeInner::Struct and the likes ; but I fear for performance.

Your idea of a "flat" DocComments with an indexmap to Spans is interesting and probably a better idea! Thanks for suggesting. I'll go in that direction.

[jimblandy]

Some more comments here.

You'll also need to change compact::compact to adjust the type handles, even though only named types can have comments and compaction never removes named types, because compaction may remove anonymous types, and that renumbers the handles.

[jimblandy]

I think this should be an Option<Box<Comments>>, so that users who aren't interested in collecting these comments don't need to spend memory on them.

[Vrixyz]

I did the change ; we could also add a feature for that ?

[jimblandy]

Maybe I didn't really say what I meant:

I think users should be able to choose to collect or ignore comments at run time. That is, naga::front::wgsl::FrontEnd::new should take an Options type, or FrontEnd should have a new_with_comments constructor - or whatever, something boring but tasteful - and then parsing should return a Module whose comments field is either populated or not.

[Vrixyz]

Thanks ! I addressed this in 57865be ; I'm open to suggestions about the implementation.

I updated a few tests to test both options.

[jimblandy]

Note: I branched off 0.22 because I'm not confident with jumping on the maybe unstable trunk.

It should be fine to work on trunk. This PR should be rebased on that.

[jimblandy]

@Vrixyz Since this isn't passing CI, to make it easier to track stuff I need to review, I'm going to convert this to a draft PR until those issues are sorted out. Feel free to flip it back into non-draft when you're ready.

[Vrixyz]

I rebased on trunk (github comment reviews are degraded, but I adressed those 🤞), tests are encouraging, there's a major bug I'm investigating still (validation_error_messages): I think spans are incorrect, which I hope to fix by working on Jim's feedback.

❓ I'm curious about "pushing rules": I imagine parsing should contain a rule for parsing comments ? so we push that rule when checking for comments.

[jimblandy]

❓ I'm curious about "pushing rules": I imagine parsing should contain a rule for parsing comments ? so we push that rule when checking for comments.

You mean Parser::rules? You shouldn't need to worry about that at all. You should just call start_byte_offset and span_from directly.

The rule stack is currently being used as a rough approximation to WGSL's template list discovery, which you definitely do not want to get tangled up with, and which shouldn't affect your work at all. And it's used for some sanity checks.

[Vrixyz]

Let's go with the Rust syntax. Can we take care of it in this PR?

I did that in 9cb6d53 👍