rsmith added inline comments.
================ Comment at: lib/AST/Stmt.cpp:457-460 this->NumOutputs = NumOutputs; this->NumInputs = NumInputs; this->NumClobbers = NumClobbers; + this->NumLabels = NumLabels; ---------------- jyu2 wrote: > rsmith wrote: > > Please assert that you don't have both outputs and labels here. (If you > > did, you would assign them the same slots within `Exprs`.) > > > > You also seem to be missing `Sema` enforcement of the rule that you cannot > > have both outputs and labels. (If you want to actually support that as an > > intentional extension to the GCC functionality, you should change the > > implementation of `GCCAsmStmt` to use different slots for outputs and > > labels, add some tests, and add a `-Wgcc-compat` warning for that case. > > Seems better to just add an error for it for now.) > This is enforcement during the parer. > > for example: > a.c:12:23: error: expected ':' > asm goto ("decl %0;" :"a": "m"(cond) : "memory" ); > > Do you think this is enough for now? > Thanks. > Jennifer Thanks, I see it now. Please still add the assert here. I'd also like a custom diagnostic for that parse error; it'd seem easy and useful to add an "asm goto cannot have output constraints" error. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits