Skip to content

full qualify Address to avoid collision with wit record of same name #960

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

Merged

Conversation

nick1udwig
Copy link
Contributor

In trying to compile Java to a WASM component, I ran into trouble because my wit file defines a record address. This collides with the codegen which frequently uses teavm Addresses.

This PR changes the codegen to fully qualify all Addresses, avoiding collision with user-defined Address classes.

@alexcrichton
Copy link
Member

Thanks! Mind adding a test in tests/codegen/*.wit for this too?

@alexcrichton alexcrichton enabled auto-merge May 31, 2024 17:22
@nick1udwig
Copy link
Contributor Author

Hmm, looks like the windows tests failed. Is this a "re-run and it'll work" scenario or should I take a second look at my test wit file?

@alexcrichton
Copy link
Member

I think this is a retry situation, although I already retried once and it failed that time too... In any case I don't believe this is related to this PR so I'll try to take it from here

@alexcrichton
Copy link
Member

It appears this is less flaky than expected, I've pinged @dicej who might be able to help here a bit more on Monday with this.

@dicej
Copy link
Collaborator

dicej commented Jun 3, 2024

The "return pointer not aligned" issue was intended to be addressed in bcd136e, but it looks like there's still a problem. I've pinged Scott to see what he thinks, and I'll try to debug it myself as well.

@dicej
Copy link
Collaborator

dicej commented Jun 3, 2024

I'm able to reproduce it locally; will try to debug. If I don't make progress, we'll just need to disable the C# variants runtime test for the time being.

@dicej
Copy link
Collaborator

dicej commented Jun 3, 2024

The generated code appears to be correct AFAICT (e.g. using ulong for 64-bit alignment), so maybe something's off at a lower level. For the time being, I'm disabling that test: #963

@alexcrichton alexcrichton added this pull request to the merge queue Jun 3, 2024
Merged via the queue into bytecodealliance:main with commit 2645c4f Jun 3, 2024
12 checks passed
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