Skip to content

Conversation

@waywardmonkeys
Copy link
Contributor

No description provided.

@waywardmonkeys
Copy link
Contributor Author

This is a precursor to trying to remove the actual definitions again. I already submitted changes to SDL3 and they were back ported to SDL2. But at least we can clean up this part now...

@kripken kripken merged commit 159b05b into emscripten-core:main Feb 21, 2023
@waywardmonkeys waywardmonkeys deleted the remove-deprecated-em-asm-variant-usages branch February 22, 2023 03:42
@sbc100
Copy link
Collaborator

sbc100 commented Feb 22, 2023

Do we flag these as deprecated at build time using some kind of clang attribute? Either way I wonder if its worth adding a test specifically for the deprecated name, at least until they are actually removed?

@kripken
Copy link
Member

kripken commented Feb 22, 2023

I didn't see a clang attribute on these, but in the header they are described as deprecated,

// Old forms for compatibility, no need to use these.
// Replace EM_ASM_, EM_ASM_ARGS and EM_ASM_INT_V with EM_ASM_INT,
// and EM_ASM_DOUBLE_V with EM_ASM_DOUBLE.
#define EM_ASM_(code, ...) emscripten_asm_const_int(CODE_EXPR(#code) _EM_ASM_PREP_ARGS(__VA_ARGS__))
#define EM_ASM_ARGS(code, ...) emscripten_asm_const_int(CODE_EXPR(#code) _EM_ASM_PREP_ARGS(__VA_ARGS__))
#define EM_ASM_INT_V(code) EM_ASM_INT(code)
#define EM_ASM_DOUBLE_V(code) EM_ASM_DOUBLE(code)

A test for this sgtm, though I could go either way as in the worst case if we break this the advice we'd give the user reporting it is to use the new EM_ASM forms. So it's not silent breakage, and only affects legacy code. (But if we get such a report, that would indicate something is missing from our docs I guess...).

@sbc100
Copy link
Collaborator

sbc100 commented Feb 22, 2023

I don't think we necessarily need test for these. It would be cool if there was some way to have clang complain when they are used though.. (I seem to remember such as thing exists, but I don't remember it of the top of my head).

@kripken
Copy link
Member

kripken commented Feb 22, 2023

There is deprecated, maybe you mean that? Maybe we can use those, but these are #defines as quoted above so we'd need to add a layer of indirection I guess, so there is a function to put the attribute on.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 23, 2023

There is deprecated, maybe you mean that? Maybe we can use those, but these are #defines as quoted above so we'd need to add a layer of indirection I guess, so there is a function to put the attribute on.

Indeed, I was hoping for a way to mark pre-processor macro as deprecated.. (IIRC it does exist).

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