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

Reply via email to