Skip to content

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 23, 2025

The type must change to unreachable in this case: the ref.get_desc
will not return a descriptor of the proper type, and we trap.

Found by #7913

@kripken kripken requested a review from tlively September 23, 2025 15:46
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

I don't get why this is necessary. If ref.get_desc would return a null, then the original allocation that installed the null descriptor would already have trapped, so the ref.get_desc will never be reached.

@kripken
Copy link
Member Author

kripken commented Sep 23, 2025

But the type is not right. We cannot return ref.null for a place that expects (ref (exact $desc)).

;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (local.set $1
;; CHECK-NEXT: (ref.as_non_null
;; CHECK-NEXT: (ref.null none)
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 traps, as you say. But the ref.null below is what determines the type of the entire expression, and it is wrong without the unreachable this PR appends.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see, thanks. So another solution would be to add a ref.as_non_null, but adding unreachable is no more complex and gives better results.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could ref.as_non_null work? It just affects nullability, but the problem is we can't return an exact descriptor since we don't have one.

Copy link
Member

Choose a reason for hiding this comment

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

But none <: (exact $desc), so the only problem here is the nullability.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, I forgot exact types do have one subtype...

@kripken kripken merged commit 7adee24 into WebAssembly:main Sep 23, 2025
16 checks passed
@kripken kripken deleted the h2l.nulldesc branch September 23, 2025 18:57
kripken added a commit that referenced this pull request Sep 25, 2025
…7921)

#7915 only handled literal nulls, but the type can be nullable too.

Simplify this by just storing a non-nullable type.
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