1253877 - Baldr: print missing text labels in resolving phase

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: better-text-errors

Comment 1

[Michael Bebenita [:mbx]]

Comment on attachment 8727125 [details] [diff] [review]
better-text-errors

Review of attachment 8727125 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, and nice to have. We could improve on this by also printing the text position of the label that fails to resolve.

::: js/src/asmjs/WasmText.cpp
@@ +3179,3 @@
>          return false;
>      }
> +    bool fail(const char* kind, WasmName name) {

NIT, since this method is specific to labels not being found, maybe we could call it something else. But I can't really think of a good name right now.

Comment 2

[Luke Wagner [:luke]]

> Looks great, and nice to have. We could improve on this by also printing the
> text position of the label that fails to resolve.

That's a good idea, but probably in a followup patch since it involves additionally stashing line/column offset in WasmName.

> NIT, since this method is specific to labels not being found, maybe we could
> call it something else. But I can't really think of a good name right now.

Good point; I'm thinking failResolveLabel().

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c9739f8236f

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/56aed3cbc3c0

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2c9739f8236f
https://hg.mozilla.org/mozilla-central/rev/56aed3cbc3c0