Skip to content

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.

Co-authored-by: Alon Zakai <alonzakai@gmail.com>
@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
@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

3 participants