nickdesaulniers 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.

Indeed, I don't trip the assertion in kernel builds using outputs.  Is 
`GCCAsmStmt::setOutputsAndInputsAndClobbers` only called for modules? If so, 
why?


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

Reply via email to