HPHP Changes for Clang

More changes for HPHP to help make it clang friendly

~~~hphp/compiler/expression/constant_expression.h
~~~hphp/compiler/expression/function_call.h
  rfind returns a size_t/unsigned int

~~~hphp/runtime/base/server/http_protocol.cpp
  Switched to std::to_string. Assuming [] was not intended here

~~~hphp/runtime/base/ref_data.h
  These fields were accessed in a public manner, assuming public was intended
  instead of private

~~~hphp/runtime/base/variable_serializer.cpp
  Switched to using [] and & to make clang happy. Assuming this was to either
  take or drop the first char.

~~~hphp/runtime/ext/asio/asio_external_thread_event_queue.h
~~~hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp
  Cast  which performs the conversions of a reinterpret_cast is not
  allowed in a constant expression. This is been moved to a macro as a temporary
  fix.

+++hphp/runtime/ext/ext_misc.cpp
  Added std::atomic to supress warnings

~~~hphp/runtime/vm/jit/simplifier.cpp
  Chosen constructor is explicit in copy-initialization

~~~hphp/runtime/vm/jit/translator-asm-helpers.S
  Ambiguous instructions require an explicit suffix
  Changed cmp to cmpl

~~~hphp/runtime/vm/jit/translator-x64-helpers.cpp
  Clang does not support global register variables

+++hphp/runtime/vm/unwind.cpp
  describeFault was only used when DEBUG or USE_TRACE was defined

~~~hphp/runtime/vm/verifier/check_unit.cpp
  Made fmt pointer const to avoid string format issues/warnings

~~~hphp/util/stack_trace.cpp
  Clang does not support variable-length arrays.
  Uniqe_ptr is used instead to take advantage runtime-sized arrays, a
  restriced form of variable-length arrays

~~~hphp/util/thread_local.h
  Clang seems to be supporting the __thread attribute, or at the very least
  it is not complaining about it.

~~~hphp/util/tiny_vector.h
  Clang does not like the flexible array here, since T is not always POD.
  I have reimplemented the array here by just sticking one value in the struct
  and calculating the offset from its address manually.
  Alterinatively, we could change the the non-POD types to be pointers, or
  we could edit their implemenations.

+++hphp/util/util.h
  Created a template for the union,
  A function declared with the constexpr specifier cannot contain
  type declarations that do not define classes or enumerations

+++hphp/runtime/vm/jit/x64-util.h
  Added a TODO
  The way hphp/runtime/vm/jit/x64-util.h is currently implemented, it only
  works if USE_GCC_FAST_TLS is defined

hphp/compiler/expression/constant_expression.h

@@ -53,7 +53,7 @@ public:
   const std::string &getName() const { return m_name;}
   const std::string &getOriginalName() const { return m_origName;}
   const std::string getNonNSOriginalName() const {
-    int nsPos = m_origName.rfind('\\');
+    auto nsPos = m_origName.rfind('\\');
     if (nsPos == string::npos) {
       return m_origName;
     }

hphp/compiler/expression/function_call.h

@@ -48,7 +48,7 @@ public:
   const std::string &getName() const { return m_name; }
   const std::string &getOriginalName() const { return m_origName; }
   const std::string getNonNSOriginalName() const {
-    int nsPos = m_origName.rfind('\\');
+    auto nsPos = m_origName.rfind('\\');
     if (nsPos == string::npos) {
       return m_origName;
     }

hphp/runtime/base/ref_data.h

@@ -104,7 +104,7 @@ private: TypedValue m_tv;
   };
 #else
 // overlap TypedValue with count
-private:
+public:
   union {
     struct {
       void* _ptr;

hphp/runtime/base/server/http_protocol.cpp

@@ -337,7 +337,7 @@ void HttpProtocol::PrepareSystemVariables(Transport *transport,
 
   // Need to append port
   if (!transport->isSSL() && RuntimeOption::ServerPort != 80) {
-    port_suffix = ":" + RuntimeOption::ServerPort;
+    port_suffix = folly::format(":{}", RuntimeOption::ServerPort).str();
   }
   server.set(s_SCRIPT_URI,
              String(prefix + (hostHeader.empty() ? hostName + port_suffix :

hphp/runtime/base/variable_serializer.cpp

@@ -657,7 +657,7 @@ void VariableSerializer::writePropertyKey(CStrRef prop) {
       int l = strlen(cls);
       m_buf->append(cls + l + 1, kl - l - 2);
       int o = m_type == PrintR ? 1 : 0;
-      m_buf->append("\":\"" + o, 3 - 2*o);
+      m_buf->append(&"\":\""[o], 3 - 2*o);
       m_buf->append(cls, l);
       const char priv[] = "\":private";
       m_buf->append(priv + o, sizeof(priv) - 1 - o);

hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp

@@ -66,7 +66,7 @@ bool AsioExternalThreadEventQueue::abandonAllReceived(c_ExternalThreadEventWaitH
 bool AsioExternalThreadEventQueue::tryReceiveSome() {
   assert(!m_received);
   m_received = m_queue.exchange(nullptr);
-  assert(m_received != k_consumerWaiting);
+  assert(m_received != K_CONSUMER_WAITING);
   return m_received;
 }
 
@@ -79,7 +79,7 @@ void AsioExternalThreadEventQueue::receiveSome() {
   // try receive external thread events without grabbing lock
   m_received = m_queue.exchange(nullptr);
   if (m_received) {
-    assert(m_received != k_consumerWaiting);
+    assert(m_received != K_CONSUMER_WAITING);
     return;
   }
 
@@ -87,18 +87,18 @@ void AsioExternalThreadEventQueue::receiveSome() {
   std::unique_lock<std::mutex> lock(m_queueMutex);
 
   // transition from empty to WAITING
-  if (m_queue.compare_exchange_strong(m_received, k_consumerWaiting)) {
+  if (m_queue.compare_exchange_strong(m_received, K_CONSUMER_WAITING)) {
     // wait for transition from WAITING to non-empty
     do {
       m_queueCondition.wait(lock);
-    } while (m_queue.load() == k_consumerWaiting);
+    } while (m_queue.load() == K_CONSUMER_WAITING);
   } else  {
     // external thread transitioned from empty to non-empty while grabbing lock
   }
 
   m_received = m_queue.exchange(nullptr);
   assert(m_received);
-  assert(m_received != k_consumerWaiting);
+  assert(m_received != K_CONSUMER_WAITING);
 }
 
 /**
@@ -107,7 +107,7 @@ void AsioExternalThreadEventQueue::receiveSome() {
 void AsioExternalThreadEventQueue::send(c_ExternalThreadEventWaitHandle* wait_handle) {
   auto next = m_queue.load();
   while (true) {
-    while (next != k_consumerWaiting) {
+    while (next != K_CONSUMER_WAITING) {
       wait_handle->setNextToProcess(next);
       if (m_queue.compare_exchange_weak(next, wait_handle)) {
         return;

hphp/runtime/ext/asio/asio_external_thread_event_queue.h

@@ -28,6 +28,12 @@ namespace HPHP {
 
 FORWARD_DECLARE_CLASS_BUILTIN(ExternalThreadEventWaitHandle);
 
+/* This is not an optimal solution
+ * This value is in principle a constexp, but the integer-to-pointer cast would
+ * require a reinterpret cast, which is not allowed in a constexpr
+ */
+#define K_CONSUMER_WAITING (static_cast<c_ExternalThreadEventWaitHandle*>((void*)1L))
+
 class AsioExternalThreadEventQueue {
   public:
     AsioExternalThreadEventQueue();
@@ -41,8 +47,6 @@ class AsioExternalThreadEventQueue {
     void send(c_ExternalThreadEventWaitHandle* wait_handle);
 
   private:
-    static constexpr auto k_consumerWaiting = static_cast<c_ExternalThreadEventWaitHandle*>((void*)1L);
-
     c_ExternalThreadEventWaitHandle* m_received;
     std::atomic<c_ExternalThreadEventWaitHandle*> m_queue;
     std::mutex m_queueMutex;

hphp/runtime/vm/jit/simplifier.cpp

@@ -37,7 +37,7 @@ StackValueInfo getStackValue(SSATmp* sp, uint32_t index) {
 
   switch (inst->op()) {
   case DefSP:
-    return {};
+    return StackValueInfo();
 
   case ReDefGeneratorSP:
   case StashGeneratorSP:

hphp/runtime/vm/jit/translator-asm-helpers.S

@@ -52,7 +52,7 @@ enterTCHelper:
    * in the ActRec pointed to by r15.  The 0x1 in the cmp instruction
    * must be kept in sync with REQ_BIND_CALL in abi-x64.h.
    */
-  cmp $0x1, 0x0(%rcx)
+  cmpl $0x1, 0x0(%rcx)
   jne .LenterTCHelper$jumpToTC
   lea .LenterTCHelper$serviceReqLabel(%rip), %rax
   push %rax
@@ -104,4 +104,4 @@ enterTCHelper:
 enterTCHelper:
   brk 0
 
-#endif
+#endif

hphp/runtime/vm/jit/translator-x64-helpers.cpp

@@ -38,7 +38,7 @@ checkEval(HPHP::Eval::PhpFile* efile) {
  * like any other local. So I put this function in its own
  * file to avoid impacting the rest of translator-x64.cpp
  */
-#ifdef __x86_64__
+#if defined(__x86_64__) && !defined(__clang__)
 register Cell* sp asm("rbx");
 #else
 // TODO(#2056140): we have to decide regalloc conventions for ARM

hphp/runtime/vm/jit/x64-util.h

@@ -56,6 +56,7 @@ emitTLSLoad(X64Assembler& a, const void* datum, RegNumber reg) {
   a.    load_disp32_reg64(virtualAddress, reg);
 }
 
+//TODO(2525714) Will not work without USE_GCC_FAST_TLS being defined
 template<typename T>
 static inline void
 emitTLSLoad(X64Assembler& a, const ThreadLocalNoCheck<T>& datum,

hphp/runtime/vm/unwind.cpp

@@ -35,7 +35,7 @@ using boost::implicit_cast;
 namespace {
 
 //////////////////////////////////////////////////////////////////////
-
+#if (defined(DEBUG) || defined(USE_TRACE))
 std::string describeFault(const Fault& f) {
   switch (f.m_faultType) {
   case Fault::UserException:
@@ -47,6 +47,7 @@ std::string describeFault(const Fault& f) {
   }
   not_reached();
 }
+#endif
 
 void discardStackTemps(const ActRec* const fp,
                        Stack& stack,

hphp/runtime/vm/verifier/check_unit.cpp

@@ -42,7 +42,7 @@ class UnitChecker {
 
  private:
   template<class... Args>
-  void error(const char* fmt, Args&&... args) {
+  void error(const char* const fmt, Args&&... args) {
     verify_error(m_unit, nullptr, fmt, std::forward<Args>(args)...);
   }
 

hphp/util/stack_trace.cpp

@@ -175,10 +175,10 @@ void StackTraceNoHeap::printStackTrace(int fd) const {
   // m_btpointers_cnt must be an upper bound on the number of filenames
   // then *2 for tolerable hash table behavior
   unsigned int bfds_size = m_btpointers_cnt * 2;
-  NamedBfd bfds[bfds_size];
-  for (unsigned int i = 0; i < bfds_size; i++) bfds[i].key[0]='\0';
+  const std::unique_ptr<NamedBfd[]> bfds (new NamedBfd[bfds_size]);
+  for (unsigned int i = 0; i < bfds_size; i++) bfds.get()[i].key[0]='\0';
   for (unsigned int i = 0; i < m_btpointers_cnt; i++) {
-    if (Translate(fd, m_btpointers[i], frame, bfds, bfds_size)) {
+    if (Translate(fd, m_btpointers[i], frame, bfds.get(), bfds_size)) {
       frame++;
     }
   }

hphp/util/thread_local.h

@@ -26,14 +26,17 @@
 namespace HPHP {
 
 ///////////////////////////////////////////////////////////////////////////////
-// Only gcc >= 4.3.0 supports the '__thread' keyword for thread locals
+// gcc >= 4.3.0 supports the '__thread' keyword for thread locals
+//
+// Clang seems to have added this feature, or at the very least it is ignoring
+// __thread keyword and compiling anyway
 //
 // icc 13.0.0 appears to support it as well but we end up with
 // assembler warnings of unknown importance about incorrect section
 // types
 
 #if !defined(NO_TLS) &&                                         \
-  ((__llvm__ && !__clang__) ||                                  \
+  ((__llvm__ && __clang__) ||                                  \
    __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 3) ||     \
    __INTEL_COMPILER)
 #define USE_GCC_FAST_TLS

hphp/util/tiny_vector.h

@@ -158,7 +158,7 @@ struct TinyVector : private boost::noncopyable {
 private:
   struct HeapData {
     uint32_t capacity; // numbers of vals---excludes this capacity field
-    T vals[];
+    T vals[0];
   };
 
   T* location(size_t index) {
@@ -215,4 +215,3 @@ private:
 }
 
 #endif
-

hphp/util/util.h

@@ -334,13 +334,15 @@ int64_t getVTableOffset(MethodPtr meth) {
   return u.offset - 1;
 }
 
+template <typename MethodPtr>
+union MethodPtrU {
+  MethodPtr meth;
+  void* ptr;
+};
+
 template <typename MethodPtr>
 constexpr void* getMethodPtr(MethodPtr meth) {
-  union U {
-    MethodPtr meth;
-    void* ptr;
-  };
-  return ((U*)(&meth))->ptr;
+  return ((MethodPtrU<MethodPtr>*)(&meth))->ptr;
 }
 
 /**