Skip to content
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

J2CL: use a more future proof naming convention for once functions #6173

Merged
merged 5 commits into from
Dec 13, 2023

Conversation

gkdn
Copy link
Contributor

@gkdn gkdn commented Dec 13, 2023

Existing convention uses _@once@_ but we also use @ for class separation.
It is cleaner&more future proof to use something other convention like _<once>_.

@gkdn
Copy link
Contributor Author

gkdn commented Dec 13, 2023

@kripken renamed _@once@_ to _<once>_ since we decided to use that convention instead.

@kripken
Copy link
Member

kripken commented Dec 13, 2023

Looks like that didn't work. From the error I'm not sure what's wrong.

Unfortunately I don't have much experience with either windows or the lit test runner that is used here, so I don't have any good guesses aside from "likely an escaping issue related to windows". So we might need to debug on a windows machine if we can't find a workaround.

However @tlively perhaps your knowledge of lit can help here - are there best practices from LLVM for avoiding windows-specific escaping issues?

@gkdn
Copy link
Contributor Author

gkdn commented Dec 13, 2023

@kripken I added a workaround with a note PTAL.

@kripken kripken enabled auto-merge (squash) December 13, 2023 21:47
@kripken kripken merged commit 61c3666 into WebAssembly:main Dec 13, 2023
14 checks passed
@tlively
Copy link
Member

tlively commented Dec 14, 2023

I don't have a fix for the Windows escaping off the top of my head. The first thing I would try is doubling up on the backslashes and if that doesn't work I would debug on a Windows box. This workaround lgtm, though.

radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
…ebAssembly#6173)

Existing convention uses _@once@_ but we also use @ for class separation.
It is cleaner&more future proof to use something other convention like _<once>_.
@gkdn gkdn mentioned this pull request Aug 31, 2024
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