meikeb added a comment.
I explained why I chose the names that you commented on. Feel free to add your
thoughts if you still think another name would be more fitting.
================
Comment at: lib/Sema/SemaChecking.cpp:3842
@@ -3841,2 +3841,3 @@
-static void CheckFormatString(Sema &S, const StringLiteral *FExpr,
+static void sumUpStringLiteralOffset(llvm::APSInt &Offset, llvm::APSInt Addend,
+ BinaryOperatorKind BinOpKind,
----------------
srhines wrote:
> Is "computeStringLiteralOffset" or "calculate..." a better name here?
I thought about that but decided to go with sumUp because compute or calculate
sounds like this function would do what we actually do what the caller of this
function does (computing the offset). This is just a nice helper to sum up the
offset we already have with another piece of offset.
================
Comment at: lib/Sema/SemaChecking.cpp:3844
@@ +3843,3 @@
+ BinaryOperatorKind BinOpKind,
+ bool AddendIsRight) {
+ unsigned BitWidth = Offset.getBitWidth();
----------------
srhines wrote:
> Is "Operand" better than "Addend"? In particular, there is the possibility
> that we do subtraction of the value instead of addition, so "Addend" makes it
> a bit confusing. Of course, I then would expect "OperandIsRight" instead of
> "AddendIsRight" too.
Clang summarizes sub and add as "additive" operands. This is why I think this
is fitting. Operand is misleading because it includes a lot more operands than
add and sub imo.
================
Comment at: lib/Sema/SemaChecking.cpp:3860
@@ +3859,3 @@
+
+ bool Ov = false;
+ llvm::APSInt ResOffset = Offset;
----------------
srhines wrote:
> Ov -> Overflow
I named that in compliance with clang naming. E.g. sadd_ov. It is common in
this file to abbreviate variable names with 1-3 characters.
https://reviews.llvm.org/D23820
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
