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 ReadModule(), which reads full binary module #35

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

binji
Copy link
Member

@binji binji commented Dec 2, 2020

Previously the only way to read a module was lazily, section by section.
That's still the most efficient way to read a module, but if you know
that you'll need the full contents anyway, it's nicer to have a function
that reads all the sections into their components directly.

This change renames ReadModule to ReadLazyModule, and introduces a
new ReadModule function that reads into the existing binary::Module
struct.

This required a few other changes:

  • The binary visitor was accidentally removing some of the locations;

  • Some module reading errors are only logged to the wasp::Errors
    struct, so you can't easily know whether reading the module succeeded
    unless you query that struct. The TextErrors and BinaryErrors
    derived classes already had this ability (has_error()), so now it is a
    virtual method on wasp::Errors too (renamed to HasError())

Previously the only way to read a module was lazily, section by section.
That's still the most efficient way to read a module, but if you know
that you'll need the full contents anyway, it's nicer to have a function
that reads all the sections into their components directly.

This change renames `ReadModule` to `ReadLazyModule`, and introduces a
new `ReadModule` function that reads into the existing `binary::Module`
struct.

This required a few other changes:

* The binary visitor was accidentally removing some of the locations;

* Some module reading errors are only logged to the `wasp::Errors`
  struct, so you can't easily know whether reading the module succeeded
  unless you query that struct. The `TextErrors` and `BinaryErrors`
  derived classes already had this ability (`has_error()`), so now it is a
  virtual method on `wasp::Errors` too (renamed to `HasError()`)
@binji binji requested a review from sbc100 December 2, 2020 20:33

protected:
void HandlePushContext(Location loc, string_view desc);
void HandlePopContext();
void HandleOnError(Location loc, string_view message);
Copy link
Member

Choose a reason for hiding this comment

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

Do we not have warning that requires override ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess not, seems that -Wsuggest-override will do it though.

@binji binji merged commit 584dffd into master Dec 3, 2020
@binji binji deleted the read-binary-module branch December 3, 2020 20:25
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.

2 participants