[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

segher added a comment. In D118297#3285571 , @MaskRay wrote: > The `volatile` qualifier is implied but makes the intention explicit. I agree > that the user should not be punished (-Wasm-volatile) by writing `asm > volatile("int3")` instead of `asm("int

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

MaskRay added a comment. The `volatile` qualifier is implied but makes the intention explicit. I agree that the user should not be punished (-Wasm-volatile) by writing `asm volatile("int3")` instead of `asm("int3")`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

jyknight added a comment. I agree that's the expected semantics. I think those semantics are unfortunate, but they're not gonna change. IMO it would've been better if you had to opt-in to "no side effects" via `__attribute__((const))` or so. But I wonder why you think we should be discouraging

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

nickdesaulniers updated this revision to Diff 404676. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - fix volatile location, more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118297/new/ https://review

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:31 +def warn_asm_volatile_goto : Warning<"volatile qualifier implied by goto">, InGroup; +def warn_asm_volatile : Warning<"volatile qualifier implied by lack of outputs">, InGro

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

nickdesaulniers added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:788 +else if (NumOutputs == 0) + Diag(Loc, diag::warn_asm_volatile); + } `Loc` is not correct if `volatile` is not the first asm-qualifier. Repository: rG LLVM Githu

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

nickdesaulniers created this revision. Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, sunfish, simoncook, johnrusso, rbar, asb, dylanmc