Skip to content

Conversation

@tlively
Copy link
Member

@tlively tlively commented Dec 9, 2023

No description provided.

@tlively
Copy link
Member Author

tlively commented Dec 9, 2023

std::optional<double> takeF64();
std::optional<float> takeF32();
std::optional<std::string_view> takeString();
std::optional<std::string> takeString();
Copy link
Member

Choose a reason for hiding this comment

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

Why did this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

The string view this method returned was dangling whenever the string contained an escape sequence, so I had to change it to return an owned string to avoid ownership problems.

;; CHECK-NEXT: )
(func $string-new (param i32 i32) (result stringref)
local.get 0
local.get 1
Copy link
Member

Choose a reason for hiding this comment

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

Why do I see local.get 1 in the input but not in the output?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was a bug! Fixed now.

@tlively tlively merged commit e795b72 into parser-array-segment-ops Dec 12, 2023
@tlively tlively deleted the parser-stringref branch December 12, 2023 01:56
tlively added a commit that referenced this pull request Dec 13, 2023
Parse `array.new_elem`, `array.init_data`, and `array.init_elem`.

Accidentally also includes:

* [Parser] Parse string types and operations (#6161)
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
Parse `array.new_elem`, `array.init_data`, and `array.init_elem`.

Accidentally also includes:

* [Parser] Parse string types and operations (WebAssembly#6161)
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