handle unknown subsections in the name section #328
Conversation
mangas
force-pushed
the
handle-unknown-sections
branch
from
5b14327
to
cc7ce21
Compare
|
Please satisfy the CI. The PR contains a lot of noise by pointless stylistic changes. |
mangas
force-pushed
the
handle-unknown-sections
branch
2 times, most recently
from
8898190
to
ba5922f
Compare
|
@athei I think I fixed the remaining ones, can you approve the CI run again please? |
mangas
force-pushed
the
handle-unknown-sections
branch
from
ba5922f
to
2524bb6
Compare
|
This is basically about new subsections in the name section that are not supported are passed through verbatim without parsing? |
mangas
force-pushed
the
handle-unknown-sections
branch
from
2524bb6
to
aed19fe
Compare
Just run |
I did, I think my IDE was on the backgroudn reformatting it 😆 |
Correct |
mangas
changed the title
|
@athei any other changes needed? 🙏 |
mangas
force-pushed
the
handle-unknown-sections
branch
2 times, most recently
from
6be5cf5
to
9cc9ca2
Compare
mangas
force-pushed
the
handle-unknown-sections
branch
from
9cc9ca2
to
dacfe8d
Compare
|
@athei do you think we have enough to merge this PR? |
Yes from my point of view. Need another review, though. |
|
Is that from @pepyakin? Anyone else we can ping here? |
|
Interesting. According to the wasm spec, this is indeed an incorrect names section. Apparently, those new items are coming from the extended-name-section proposal. I guess we can ignore those for now and proceed with merging this PR. |
|
You still need to approve / merge. I can't. |
| @@ -97,7 +97,12 @@ impl NameSection { | |||
| local_names = Some(LocalNameSubsection::deserialize(module, rdr)?); | |||
| }, | |||
|
|
|||
| _ => return Err(Error::UnknownNameSubsectionType(subsection_type)), | |||
There was a problem hiding this comment.
Is this error still emitted anywhere? Perhaps would be a cleanup to drop it from the enum.
There was a problem hiding this comment.
That is a good idea if this is the case.
There was a problem hiding this comment.
seems like it was only emitted by the code we just removed. Raised #330 to remove it
|
@athei @pepyakin thanks for the help so far. Would be possible to create a new tag with the changes so I can use them in paritytech/wasm-instrument#16? |
Closes #271