void added a comment. In D88195#2293559 <https://reviews.llvm.org/D88195#2293559>, @nickdesaulniers wrote:
> In D88195#2293533 <https://reviews.llvm.org/D88195#2293533>, @void wrote: > >> In D88195#2293165 <https://reviews.llvm.org/D88195#2293165>, >> @nickdesaulniers wrote: >> >>> I'm super confused between the commit message and initial hunk, that seem >>> to make sense and probably should go in clang-11 if it's not too late, and >>> the additional tests for modules which the commit message does not address. >>> Were these meant to be separate commits, because otherwise it looks like >>> you uploaded unrelated stuff. Are C++20 modules broken with `asm goto`, or >>> are you just adding additional test coverage for that? >> >> The assert only triggers for modules, so yeah modules are broken with "asm >> goto", but only if asserts are enabled. > > The assert was removed from AST/Stmt.cpp; it doesn't look module related. > Wouldn't it be triggered by ANY `GCCAsmStmt`? (I have patches that use ASM > goto w/ outputs on the kernel, let me try an assertions enabled build). It's > not obvious to me why the method modified would only trigger for modules. Yes, but that particular function is only called during serialization reading. It would trigger for any serialization, this just happens to be for modules. I'll edit the commit message to be clearer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88195/new/ https://reviews.llvm.org/D88195 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
