eliminate many windows warnings

Summary: There are a variety of Windows (MSVC) compiler warnings that fire when building Hermes. If we can fix them (or at least quiet them), we can get into a treat-warnings-as-errors state and stay clean moving forward. This diff fixes them all in the core Hermes code. (Sadly there are still warnings in many *test files* themselves, so I can't actually enable warnings-as-errors just yet)

Reviewed By: jpporto

Differential Revision: D41671449

fbshipit-source-id: faeda555f96a755f37b4b48e4a7a41d3faa51c57

external/llvh/lib/Support/SourceMgr.cpp

@@ -126,7 +126,7 @@ std::pair<StringRef, unsigned> SourceMgr::SrcBuffer::getLineNumber(
       EOL != Offsets->end() ? BufStart + *EOL + 1 : Buffer->getBufferEnd();
 
   // Lines count from 1, so add 1 to the distance from the 0th line.
-  return {StringRef(LineStart, LineEnd - LineStart), (1 + (EOL - Offsets->begin()))};
+  return {StringRef(LineStart, LineEnd - LineStart), static_cast<unsigned>((1 + (EOL - Offsets->begin())))};
 }
 
 template<typename T>
@@ -219,7 +219,7 @@ std::pair<unsigned, unsigned>
 SourceMgr::getLineAndColumn(SMLoc Loc, unsigned BufferID) const {
   auto LineRefAndNo = FindLine(Loc, BufferID);
   return std::make_pair(LineRefAndNo.second,
-                        Loc.getPointer() - LineRefAndNo.first.data() + 1);
+                        static_cast<unsigned>(Loc.getPointer() - LineRefAndNo.first.data() + 1));
 }
 
 void SourceMgr::PrintIncludeStack(SMLoc IncludeLoc, raw_ostream &OS) const {
@@ -285,8 +285,8 @@ SMDiagnostic SourceMgr::GetMessage(SMLoc Loc, SourceMgr::DiagKind Kind,
 
       // Translate from SMLoc ranges to column ranges.
       // FIXME: Handle multibyte characters.
-      ColRanges.push_back(std::make_pair(R.Start.getPointer()-LineStart,
-                                         R.End.getPointer()-LineStart));
+      ColRanges.push_back(std::make_pair(static_cast<unsigned>(R.Start.getPointer()-LineStart),
+                                         static_cast<unsigned>(R.End.getPointer()-LineStart)));
     }
 
     LineAndCol = getLineAndColumn(Loc, CurBuf);

external/llvh/patches/SourceMgr-msvc-warnings.patch

@@ -0,0 +1,32 @@
+diff --git a/xplat/hermes/external/llvh/lib/Support/SourceMgr.cpp b/xplat/hermes/external/llvh/lib/Support/SourceMgr.cpp
+--- a/xplat/hermes/external/llvh/lib/Support/SourceMgr.cpp
++++ b/xplat/hermes/external/llvh/lib/Support/SourceMgr.cpp
+@@ -126,7 +126,7 @@
+       EOL != Offsets->end() ? BufStart + *EOL + 1 : Buffer->getBufferEnd();
+
+   // Lines count from 1, so add 1 to the distance from the 0th line.
+-  return {StringRef(LineStart, LineEnd - LineStart), (1 + (EOL - Offsets->begin()))};
++  return {StringRef(LineStart, LineEnd - LineStart), static_cast<unsigned>((1 + (EOL - Offsets->begin())))};
+ }
+
+ template<typename T>
+@@ -219,7 +219,7 @@
+ SourceMgr::getLineAndColumn(SMLoc Loc, unsigned BufferID) const {
+   auto LineRefAndNo = FindLine(Loc, BufferID);
+   return std::make_pair(LineRefAndNo.second,
+-                        Loc.getPointer() - LineRefAndNo.first.data() + 1);
++                        static_cast<unsigned>(Loc.getPointer() - LineRefAndNo.first.data() + 1));
+ }
+
+ void SourceMgr::PrintIncludeStack(SMLoc IncludeLoc, raw_ostream &OS) const {
+@@ -285,8 +285,8 @@
+
+       // Translate from SMLoc ranges to column ranges.
+       // FIXME: Handle multibyte characters.
+-      ColRanges.push_back(std::make_pair(R.Start.getPointer()-LineStart,
+-                                         R.End.getPointer()-LineStart));
++      ColRanges.push_back(std::make_pair(static_cast<unsigned>(R.Start.getPointer()-LineStart),
++                                         static_cast<unsigned>(R.End.getPointer()-LineStart)));
+     }
+
+     LineAndCol = getLineAndColumn(Loc, CurBuf);

external/zip/src/zip.h

@@ -313,7 +313,7 @@ extern int zip_entries_total(struct zip_t *zip);
  * @return the number of deleted entries, or negative number (< 0) on error.
  */
 extern int zip_entries_delete(struct zip_t *zip, char *const entries[],
-                              size_t len);
+                              const size_t len);
 
 /**
  * Extracts a zip archive stream into directory.

include/hermes/VM/SmallHermesValue-inline.h

@@ -24,10 +24,14 @@
 namespace hermes {
 namespace vm {
 
+#ifndef HERMESVM_ALLOW_COMPRESSED_POINTERS
+
 void SmallHermesValueAdaptor::setInGC(SmallHermesValueAdaptor hv, GC &gc) {
   HermesValue::setInGC(hv, gc);
 }
 
+#else // #ifndef HERMESVM_ALLOW_COMPRESSED_POINTERS
+
 void HermesValue32::setInGC(HermesValue32 hv, GC &gc) {
   setNoBarrier(hv);
   assert(gc.calledByGC());
@@ -148,6 +152,9 @@ double HermesValue32::getNumber(PointerBase &pb) const {
       "Strings must use encodeStringValue; BigInts, encodeBigIntValue");
   return encodePointerImpl(ptr, Tag::Object, pb);
 }
+
+#endif // #ifndef HERMESVM_ALLOW_COMPRESSED_POINTERS
+
 } // namespace vm
 } // namespace hermes
 #pragma GCC diagnostic pop

include/hermes/VM/SmallHermesValue.h

@@ -19,11 +19,7 @@
 #include <cassert>
 #include <cmath>
 #include <cstdint>
-#pragma GCC diagnostic push
 
-#ifdef HERMES_COMPILER_SUPPORTS_WSHORTEN_64_TO_32
-#pragma GCC diagnostic ignored "-Wshorten-64-to-32"
-#endif
 namespace hermes {
 namespace vm {
 
@@ -32,6 +28,13 @@ class StringPrimitive;
 class GCCell;
 class Runtime;
 
+/// If compressed pointers are allowed, then we should also compress
+/// HermesValues. This means that when compressed pointers are allowed,
+/// SmallHermesValue will almost always be 32 bits, except with MallocGC, which
+/// does not support compressed pointers. Depending on the compressed pointers
+/// flag, SmallHermesValue will alias SmallHermesValueAdaptor or HermesValue32.
+
+#ifndef HERMESVM_ALLOW_COMPRESSED_POINTERS
 /// An adaptor class that provides the API of a SmallHermesValue is internally
 /// just a HermesValue.
 class SmallHermesValueAdaptor : protected HermesValue {
@@ -168,6 +171,9 @@ class SmallHermesValueAdaptor : protected HermesValue {
     return SmallHermesValueAdaptor{HermesValue::encodeEmptyValue()};
   }
 };
+using SmallHermesValue = SmallHermesValueAdaptor;
+
+#else // #ifndef HERMESVM_ALLOW_COMPRESSED_POINTERS
 
 /// A compressed HermesValue that is always equal to the size of a
 /// CompressedPointer. It uses the least significant bits (guaranteed to be zero
@@ -476,18 +482,9 @@ class HermesValue32 {
     raw_ = other.raw_;
   }
 };
+using SmallHermesValue = HermesValue32;
 
-/// If compressed pointers are allowed, then we should also compress
-/// HermesValues. This means that when compressed pointers are allowed,
-/// SmallHermesValue will almost always be 32 bits, except with MallocGC, which
-/// does not support compressed pointers.
-using SmallHermesValue =
-#ifdef HERMESVM_ALLOW_COMPRESSED_POINTERS
-    HermesValue32
-#else
-    SmallHermesValueAdaptor
-#endif
-    ;
+#endif // #ifndef HERMESVM_ALLOW_COMPRESSED_POINTERS
 
 static_assert(
     std::is_trivial<SmallHermesValue>::value,
@@ -498,5 +495,4 @@ using GCSmallHermesValue = GCHermesValueBase<SmallHermesValue>;
 } // end namespace vm
 } // end namespace hermes
 
-#pragma GCC diagnostic pop
 #endif // HERMES_VM_HERMESVALUE_H

lib/BCGen/HBC/BytecodeStream.cpp

@@ -190,7 +190,8 @@ void BytecodeSerializer::serializeFunctionsBytecode(BytecodeModule &BM) {
       if (isLayout_) {
         // Deduplicate the bytecode during layout phase.
         DedupKey key = entry->getOpcodeArray();
-        auto pair = bcMap.insert(std::make_pair(key, loc_));
+        auto pair =
+            bcMap.insert(std::make_pair(key, static_cast<uint32_t>(loc_)));
         if (!pair.second) {
           reuse = true;
           entry->setOffset(pair.first->second);

lib/IR/IR.cpp

@@ -102,7 +102,7 @@ void Value::removeUse(Use U) {
   // If we've changed the location of a use in the use list then we need to
   // update the operand in the user.
   if (U.second != Users.size()) {
-    Use oldUse = {this, Users.size()};
+    Use oldUse = {this, static_cast<unsigned>(Users.size())};
     auto &operands = Users[U.second]->Operands;
     for (int i = 0, e = operands.size(); i < e; i++) {
       if (operands[i] == oldUse) {
@@ -116,7 +116,7 @@ void Value::removeUse(Use U) {
 
 Value::Use Value::addUser(Instruction *Inst) {
   Users.push_back(Inst);
-  return {this, Users.size() - 1};
+  return {this, static_cast<unsigned>(Users.size() - 1)};
 }
 
 void Value::replaceAllUsesWith(Value *Other) {

lib/Support/BigIntSupport.cpp

@@ -2015,7 +2015,8 @@ static std::tuple<uint32_t, bool> getShiftAmountAndSign(
     // shiftAmnt is outside of the
     // [MinNegativeShiftAmountInBits, MaxPositiveShiftAmountInBits]; thus return
     // a really large shift amount.
-    return std::make_tuple(reallyLargeShiftAmount, shiftAmntIsNeg);
+    return std::make_tuple(
+        static_cast<uint32_t>(reallyLargeShiftAmount), shiftAmntIsNeg);
   }
 
   const SignedBigIntDigitType sa = (shiftAmnt.numDigits == 0)
@@ -2026,7 +2027,8 @@ static std::tuple<uint32_t, bool> getShiftAmountAndSign(
        shiftAmnt.digits[0] != std::numeric_limits<BigIntDigitType>::min()) &&
       "shiftAmnt is MIN_INT, hence -signedShiftAmnt is MIN_INT");
   // Always return a positive result -- thus negate sa if shiftAmnt is negative.
-  return std::make_tuple(shiftAmntIsNeg ? -sa : sa, shiftAmntIsNeg);
+  return std::make_tuple(
+      static_cast<uint32_t>(shiftAmntIsNeg ? -sa : sa), shiftAmntIsNeg);
 }
 
 } // namespace

lib/Support/OSCompatWindows.cpp

@@ -16,7 +16,9 @@
 // Include windows.h first because other includes from windows API need it.
 // The blank line after the include is necessary to avoid lint error.
 #define WIN32_LEAN_AND_MEAN
+#ifndef NOMINMAX
 #define NOMINMAX // do not define min/max macros
+#endif
 #include <windows.h>
 
 #include <intrin.h>

lib/VM/Instrumentation/ProcessStats.cpp

@@ -11,7 +11,9 @@
 // Include windows.h first because other includes from windows API need it.
 // The blank line after the include is necessary to avoid lint error.
 #define WIN32_LEAN_AND_MEAN
+#ifndef NOMINMAX
 #define NOMINMAX // do not define min/max macros
+#endif
 #include <windows.h>
 
 #include <psapi.h>

lib/VM/JSLib/Math.cpp

@@ -17,7 +17,9 @@
 #include "hermes/VM/SingleObject.h"
 #include "hermes/VM/StringPrimitive.h"
 
+#ifndef _USE_MATH_DEFINES
 #define _USE_MATH_DEFINES
+#endif
 #include <float.h>
 #include <math.h>
 #include <random>

lib/VM/Operations.cpp

@@ -1104,11 +1104,11 @@ abstractEqualityTest_RJS(Runtime &runtime, Handle<> xHandle, Handle<> yHandle) {
       // ToNumber(x) == y.
       CASE_S_S(Bool, NUMBER_TAG) {
         // Do both conversions and check numerical equality.
-        return x->getBool() == y->getNumber();
+        return static_cast<double>(x->getBool()) == y->getNumber();
       }
       CASE_S_M(Bool, Str) {
         // Do string parsing and check double equality.
-        return x->getBool() ==
+        return static_cast<double>(x->getBool()) ==
             stringToNumber(runtime, Handle<StringPrimitive>::vmcast(y));
       }
       CASE_S_M(Bool, BigInt) {
@@ -1121,11 +1121,11 @@ abstractEqualityTest_RJS(Runtime &runtime, Handle<> xHandle, Handle<> yHandle) {
       // 9. If Type(y) is Boolean, return the result of the comparison x == !
       // ToNumber(y).
       CASE_S_S(NUMBER_TAG, Bool) {
-        return x->getNumber() == y->getBool();
+        return x->getNumber() == static_cast<double>(y->getBool());
       }
       CASE_M_S(Str, Bool) {
         return stringToNumber(runtime, Handle<StringPrimitive>::vmcast(x)) ==
-            y->getBool();
+            static_cast<double>(y->getBool());
       }
       CASE_M_S(BigInt, Bool) {
         return x->getBigInt()->compare(static_cast<int32_t>(y->getBool())) == 0;

lib/VM/StorageProvider.cpp

@@ -129,7 +129,7 @@ llvh::ErrorOr<void *> VMAllocateStorageProvider::newStorageImpl(
   }
   void *mem = *result;
   assert(isAligned(mem));
-  (void)isAligned;
+  (void)&isAligned;
 #ifdef HERMESVM_ALLOW_HUGE_PAGES
   oscompat::vm_hugepage(mem, AlignedStorage::size());
 #endif

lib/VM/StringPrimitive.cpp

@@ -97,7 +97,15 @@ CallResult<HermesValue> StringPrimitive::createEfficientImpl(
     }
     auto output = runtime.makeHandle<StringPrimitive>(*result);
     // Copy directly into the StringPrimitive storage.
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable : 4244)
+#endif
+
     std::copy(str.begin(), str.end(), output->castToASCIIPointerForWrite());
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
     return output.getHermesValue();
   }
 
@@ -200,8 +208,15 @@ CallResult<HermesValue> StringPrimitive::createDynamic(
       return ExecutionStatus::EXCEPTION;
     }
     // Copy directly into the StringPrimitive storage.
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable : 4244)
+#endif
     std::copy(
         str.begin(), str.end(), res->getString()->castToASCIIPointerForWrite());
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
     return res;
   } else {
     return DynamicUTF16StringPrimitive::create(runtime, str);