Bug 1356843 - Fix -Wcomma warnings in js/. r?jorendorff draft
authorChris Peterson <cpeterson@mozilla.com>
Sun, 09 Apr 2017 21:15:01 -0700
changeset 563968 5b5028c76ccfca2ab2cb5eea66c1c6719ef1a429
parent 563967 ac8f478a584f71bf18fe4df092c81bf9f3aeee60
child 563969 d9d54cc31e92ebf0e07aee98daff3958e040d6cc
push id54487
push usercpeterson@mozilla.com
push dateTue, 18 Apr 2017 06:09:29 +0000
reviewersjorendorff
bugs1356843
milestone55.0a1
Bug 1356843 - Fix -Wcomma warnings in js/. r?jorendorff clang's -Wcomma warning warns about suspicious use of the comma operator such as between two statements or to call a function for side effects within an expression. js/src/builtin/MapObject.cpp:786:48 [-Wcomma] possible misuse of comma operator here js/src/builtin/MapObject.cpp:1371:48 [-Wcomma] possible misuse of comma operator here js/src/builtin/RegExp.cpp:1266:62 [-Wcomma] possible misuse of comma operator here js/src/jit/x64/BaseAssembler-x64.h:624:99 [-Wcomma] possible misuse of comma operator here js/src/jsarray.cpp:2416:27 [-Wcomma] possible misuse of comma operator here js/src/jscompartment.cpp:120:48 [-Wcomma] possible misuse of comma operator here js/src/jsstr.cpp:3346:14 [-Wcomma] possible misuse of comma operator here js/xpconnect/src/XPCWrappedNativeJSOps.cpp:316:71 [-Wcomma] possible misuse of comma operator here MozReview-Commit-ID: BbT4otUXczV
js/src/builtin/MapObject.cpp
js/src/builtin/RegExp.cpp
js/src/jit/x64/BaseAssembler-x64.h
js/src/jsarray.cpp
js/src/jscompartment.cpp
js/src/jsstr.cpp
js/src/shell/js.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -778,17 +778,20 @@ MapObject::delete_(JSContext* cx, unsign
 }
 
 bool
 MapObject::iterator(JSContext* cx, IteratorKind kind,
                     HandleObject obj, MutableHandleValue iter)
 {
     ValueMap& map = extract(obj);
     Rooted<JSObject*> iterobj(cx, MapIteratorObject::create(cx, obj, &map, kind));
-    return iterobj && (iter.setObject(*iterobj), true);
+    if (!iterobj)
+        return false;
+    iter.setObject(*iterobj);
+    return true;
 }
 
 bool
 MapObject::iterator_impl(JSContext* cx, const CallArgs& args, IteratorKind kind)
 {
     RootedObject obj(cx, &args.thisv().toObject());
     return iterator(cx, kind, obj, args.rval());
 }
@@ -1363,17 +1366,20 @@ SetObject::delete_(JSContext* cx, unsign
 
 bool
 SetObject::iterator(JSContext *cx, IteratorKind kind,
                     HandleObject obj, MutableHandleValue iter)
 {
     MOZ_ASSERT(SetObject::is(obj));
     ValueSet &set = extract(obj);
     Rooted<JSObject*> iterobj(cx, SetIteratorObject::create(cx, obj, &set, kind));
-    return iterobj && (iter.setObject(*iterobj), true);
+    if (!iterobj)
+        return false;
+    iter.setObject(*iterobj);
+    return true;
 }
 
 bool
 SetObject::iterator_impl(JSContext *cx, const CallArgs& args, IteratorKind kind)
 {
     Rooted<SetObject*> setobj(cx, &args.thisv().toObject().as<SetObject>());
     ValueSet& set = *setobj->getData();
     Rooted<JSObject*> iterobj(cx, SetIteratorObject::create(cx, setobj, &set, kind));
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -1266,25 +1266,29 @@ InterpretDollar(JSLinearString* matched,
         /* $n, $nn */
         unsigned num = JS7_UNDEC(c);
         if (num > captures.length()) {
             // The result is implementation-defined, do not substitute.
             return false;
         }
 
         const CharT* currentChar = currentDollar + 2;
-        if (currentChar < replacementEnd && (c = *currentChar, JS7_ISDEC(c))) {
-            unsigned tmpNum = 10 * num + JS7_UNDEC(c);
-            // If num > captures.length(), the result is implementation-defined.
-            // Consume next character only if num <= captures.length().
-            if (tmpNum <= captures.length()) {
-                currentChar++;
-                num = tmpNum;
+        if (currentChar < replacementEnd) {
+            c = *currentChar;
+            if (JS7_ISDEC(c)) {
+                unsigned tmpNum = 10 * num + JS7_UNDEC(c);
+                // If num > captures.length(), the result is implementation-defined.
+                // Consume next character only if num <= captures.length().
+                if (tmpNum <= captures.length()) {
+                    currentChar++;
+                    num = tmpNum;
+                }
             }
         }
+
         if (num == 0) {
             // The result is implementation-defined.
             // Do not substitute.
             return false;
         }
 
         *skip = currentChar - currentDollar;
 
--- a/js/src/jit/x64/BaseAssembler-x64.h
+++ b/js/src/jit/x64/BaseAssembler-x64.h
@@ -616,17 +616,17 @@ class BaseAssemblerX64 : public BaseAsse
         }
 
         spew("movq       %p, %s", addr, GPReg64Name(dst));
         m_formatter.oneByteOp64(OP_MOV_GvEv, addr, dst);
     }
 
     void leaq_mr(int32_t offset, RegisterID base, RegisterID index, int scale, RegisterID dst)
     {
-        spew("leaq       " MEM_obs ", %s", ADDR_obs(offset, base, index, scale), GPReg64Name(dst)),
+        spew("leaq       " MEM_obs ", %s", ADDR_obs(offset, base, index, scale), GPReg64Name(dst));
         m_formatter.oneByteOp64(OP_LEA, offset, base, index, scale, dst);
     }
 
     void movq_i32m(int32_t imm, int32_t offset, RegisterID base)
     {
         spew("movq       $%d, " MEM_ob, imm, ADDR_ob(offset, base));
         m_formatter.oneByteOp64(OP_GROUP11_EvIz, offset, base, GROUP11_MOV);
         m_formatter.immediate32(imm);
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2409,17 +2409,17 @@ js::array_unshift(JSContext* cx, unsigne
             } while (false);
 
             // Steps 4.b-c.
             if (!optimized) {
                 uint32_t last = length;
                 double upperIndex = double(last) + args.length();
                 RootedValue value(cx);
                 do {
-                    --last, --upperIndex;
+                    --last; --upperIndex;
                     if (!CheckForInterrupt(cx))
                         return false;
                     bool hole;
                     if (!HasAndGetElement(cx, obj, last, &hole, &value))
                         return false;
                     if (hole) {
                         if (!DeletePropertyOrThrow(cx, obj, upperIndex))
                             return false;
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -112,17 +112,17 @@ JSCompartment::~JSCompartment()
 
     js_delete(jitCompartment_);
     js_delete(watchpointMap);
     js_delete(scriptCountsMap);
     js_delete(debugScriptMap);
     js_delete(debugEnvs);
     js_delete(objectMetadataTable);
     js_delete(lazyArrayBuffers);
-    js_delete(nonSyntacticLexicalEnvironments_),
+    js_delete(nonSyntacticLexicalEnvironments_);
     js_free(enumerators);
 
 #ifdef DEBUG
     // Avoid assertion destroying the unboxed layouts list if the embedding
     // leaked GC things.
     if (!rt->gc.shutdownCollectedEverything())
         unboxedLayouts.clear();
 #endif
@@ -1445,9 +1445,8 @@ AutoSetNewObjectMetadata::~AutoSetNewObj
         // in order.
         cx_->compartment()->objectMetadataState = prevState_;
 
         obj = SetNewObjectMetadata(cx_, obj);
     } else {
         cx_->compartment()->objectMetadataState = prevState_;
     }
 }
-
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3704,17 +3704,17 @@ js_strlen(const char16_t* s)
 int32_t
 js_strcmp(const char16_t* lhs, const char16_t* rhs)
 {
     while (true) {
         if (*lhs != *rhs)
             return int32_t(*lhs) - int32_t(*rhs);
         if (*lhs == 0)
             return 0;
-        ++lhs, ++rhs;
+        ++lhs; ++rhs;
     }
 }
 
 int32_t
 js_fputs(const char16_t* s, FILE* f)
 {
     while (*s != 0) {
         if (fputwc(wchar_t(*s), f) == WEOF)
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -2890,17 +2890,17 @@ struct DisassembleOptionParser {
             if (JS_FlatStringEqualsAscii(flatStr, "-l"))
                 lines = true;
             else if (JS_FlatStringEqualsAscii(flatStr, "-r"))
                 recursive = true;
             else if (JS_FlatStringEqualsAscii(flatStr, "-S"))
                 sourceNotes = false;
             else
                 break;
-            argv++, argc--;
+            argv++; argc--;
         }
         return true;
     }
 };
 
 } /* anonymous namespace */
 
 static bool
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -308,17 +308,17 @@ DefinePropertyIfFound(XPCCallContext& cc
             JSAutoByteString name;
             RefPtr<XPCNativeInterface> iface2;
             XPCWrappedNativeTearOff* to;
             RootedObject jso(ccx);
             nsresult rv = NS_OK;
 
             if (JSID_IS_STRING(id) &&
                 name.encodeLatin1(ccx, JSID_TO_STRING(id)) &&
-                (iface2 = XPCNativeInterface::GetNewOrUsed(name.ptr()), iface2) &&
+                (iface2 = XPCNativeInterface::GetNewOrUsed(name.ptr())) &&
                 nullptr != (to = wrapperToReflectInterfaceNames->
                            FindTearOff(iface2, true, &rv)) &&
                 nullptr != (jso = to->GetJSObject()))
 
             {
                 AutoResolveName arn(ccx, id);
                 if (resolved)
                     *resolved = true;