bsmith added inline comments.

================
Comment at: llvm/lib/IR/Attributes.cpp:570
+    Result += utostr(MinValue);
+    Result += ',';
+    Result += utostr(MaxValue);
----------------
peterwaller-arm wrote:
> Nit: The only other precedent I can see for this is `allocsize`. Grepping the 
> code I found this is always written in tests as `allocsize(x, y)`, however, 
> it prints as `allocsize(x,y)` (no space) when done with `-emit-llvm`, 
> regardless of how it was formatted as input. I figure that is an oversight 
> and this should have the space.
I'm reluctant to change `allocsize` and given that is the only other example of 
multiple parameters to an attribute like this, I'm inclined to stay consistent 
with it. I also think there is an argument to be made about not intruding extra 
spaces in a potentially already cluttered attributes section.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98030/new/

https://reviews.llvm.org/D98030

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to