Issue 4127 - [emscripten] Convert subscription classes to C++ - Part 1

compiled/bindings.ipp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 24 matching lines @@
 
 namespace bindings_internal
 {
   typedef void* TYPEID;
 
   enum class TypeCategory
   {
     UNKNOWN,
     VOID,
     INT,
+    INT64,
+    FLOAT,

[sergei, 2017/04/12 12:04:32]

I would rather call it DOUBLE because emscripten uses double (actually JS
number).

[Wladimir Palant, 2017/04/13 13:04:40]

I was under the impression that Emscripten doesn't really distinguish between
single and double precision floating point numbers. Turned out I was wrong - see
https://github.com/kripken/emscripten/blob/1.37.3/src/modules.js#L46. So we
should have both FLOAT and DOUBLE here.

     DEPENDENT_STRING,
     OWNED_STRING,
     STRING_REF,
     CLASS_PTR
   };
 
   template<typename T>
   struct TypeInfo
   {
     /*
      * Since TypeInfo is a templated type, in practice the compiler will define
      * a new type for each possible template parameter value. We use that fact
      * to generate type identifiers: each of these TypeInfo types has a
      * different s_typeIDHelper member, so we use a pointer to that static
      * variable as a type identifier - it will be different for each template
      * parameter.
      */
     static char s_typeIDHelper;
     constexpr operator TYPEID() const
     {
       return &s_typeIDHelper;
     }
 
     constexpr operator TypeCategory() const
     {
       if (std::is_void<T>())
         return TypeCategory::VOID;
 
+      if (std::is_same<T, uint64_t>())
+        return TypeCategory::INT64;
+
       if (std::is_integral<T>() || std::is_enum<T>())
         return TypeCategory::INT;
 
+      if (std::is_floating_point<T>())
+        return TypeCategory::FLOAT;
+
       if (std::is_same<DependentString, T>() || std::is_same<const DependentString, T>())
         return TypeCategory::DEPENDENT_STRING;
 
       if (std::is_same<OwnedString, T>() || std::is_same<const OwnedString, T>())
         return TypeCategory::OWNED_STRING;
 
       if (std::is_same<String&, T>() || std::is_same<const String&, T>() ||
         std::is_same<DependentString&, T>())
       {
         return TypeCategory::STRING_REF;
@@ skipping 39 matching lines @@
           instance_function(instance_function)
     {
       name[0] = '\0';
 
       // The function parameter is a pointer to the function pointer.
       // Emscripten's "function pointers" are actually integers indicating the
       // position in the call table. 0 represents nullptr.
       if (!*reinterpret_cast<int*>(function))
         return;
 
+      std::string signature;
+      if (returnType == TypeCategory::DEPENDENT_STRING ||
+          returnType == TypeCategory::OWNED_STRING)
+      {
+        // Objects aren't really returned but passed as parameter. Note that
+        // this pointer might come before it but we don't care because both
+        // are integers (pointers) as far as Emscripten is concerned.
+        signature += "vi";
+      }
+      else if (returnType == TypeCategory::VOID)
+        signature += 'v';
+      else if (returnType == TypeCategory::FLOAT)
+        signature += 'd';
+      else if (returnType == TypeCategory::INT ||
+          returnType == TypeCategory::INT64 ||
+          returnType == TypeCategory::STRING_REF ||
+          returnType == TypeCategory::CLASS_PTR)
+      {
+        signature += 'i';
+      }
+      else
+        throw std::runtime_error("Unexpected function return type");
+
+      if (instance_function)
+      {
+        // this pointer is an implicit parameter
+        signature += 'i';
+      }
+
       for (const auto& item : argTypes)
       {
-        if (item != TypeCategory::INT && item != TypeCategory::STRING_REF &&
-            item != TypeCategory::CLASS_PTR)
+        if (item == TypeCategory::INT || item == TypeCategory::STRING_REF ||
+            item == TypeCategory::CLASS_PTR)
         {
+          signature += 'i';
+        }
+        else if (item == TypeCategory::INT64)
+          signature += "ii";

[sergei, 2017/04/12 12:04:32]

It would be good to have a link (maybe better to some code, commit/file:line),
from which one can understand that we are passing correct values to
Runtime.dynCall.

[Wladimir Palant, 2017/04/13 13:04:39]

Ok, I managed to find the correct code line, linked to it.

+        else if (item == TypeCategory::FLOAT)
+          signature += 'd';
+        else
           throw std::runtime_error("Unexpected function argument type");
-        }
         args.push_back(item);
       }
 
-      if (returnType != TypeCategory::VOID && returnType != TypeCategory::INT &&
-          returnType != TypeCategory::DEPENDENT_STRING &&
-          returnType != TypeCategory::OWNED_STRING &&
-          returnType != TypeCategory::STRING_REF &&
-          returnType != TypeCategory::CLASS_PTR)
-      {
-        throw std::runtime_error("Unexpected function return type");
-      }
-
-      effectiveArgs = args.size();
-      effectiveReturnType = returnType;
-      if (instance_function)
-        effectiveArgs++;
-
-      if (returnType == TypeCategory::DEPENDENT_STRING ||
-          returnType == TypeCategory::OWNED_STRING)
-      {
-        effectiveArgs++;
-        effectiveReturnType = TypeCategory::VOID;
-      }
-
-      get_function_name(function, effectiveArgs,
-          effectiveReturnType == TypeCategory::VOID);
+      get_function_name(function, signature.c_str());
     }
 
     template<typename ReturnType, typename... Args>
     FunctionInfo(ReturnType (*function)(Args...))
         : FunctionInfo(TypeInfo<ReturnType>(),
           TypeInfo<ReturnType>().pointer_type(), { TypeInfo<Args>()... }, false,
           &function)
     {
     }
 
@@ skipping 11 matching lines @@
           TypeInfo<ReturnType>().pointer_type(), { TypeInfo<Args>()... }, true,
           &function)
     {
     }
 
     bool empty() const
     {
       return name[0] == '\0';
     }
 
-    void get_function_name(void* ptr, int numArgs, bool voidResult)
+    void get_function_name(void* ptr, const char* signature)
     {
       // This is a hack, C++ won't let us get the mangled function name.
       // JavaScript is more dynamic so we pass the pointer to our function
       // there. With that and the function signature we can call the function -
       // with a full stack so that we will cause it to abort. Sometimes the
       // function we are calling will also be missing from the build. The result
       // is the same: abort() is called which in turn calls stackTrace(). By
       // replacing stackTrace() we get access to the call stack and search it
       // for the name of our function.
 
       EM_ASM_ARGS({
-        var signature = $3 ? "v" : "i";
+        var signature = AsciiToString($2);
         var args = [];
-        for (var i = 0; i < $2; i++)
-        {
-          signature += "i";
+        for (var i = 1; i < signature.length; i++)
           args.push(0);
-        }
 
         var oldPrint = Module.print;
         var oldPrintErr = Module.printErr;
         var oldStackTrace = stackTrace;
         var sp = Runtime.stackSave();
         Module.print = function(){};
         Module.printErr = function(){};
         stackTrace = function()
         {
           var stack = [];
@@ skipping 24 matching lines @@
         {
           Module.stringToAscii(e, $0);
         }
         finally
         {
           Runtime.stackRestore(sp);
           Module.print = oldPrint;
           Module.printErr = oldPrintErr;
           stackTrace = oldStackTrace;
         }
-      }, name, ptr, numArgs, voidResult);
+      }, name, ptr, signature);
     }
   };
 
   class NoBaseClass
   {
   };
 
   struct PropertyInfo
   {
     std::string name;
@@ skipping 119 matching lines @@
     for (int i = 0; i < params.size(); i++)
     {
       if (i > 0)
         call_str += ", ";
       call_str += params[i];
     }
     call_str += ")";
 
     if (call.returnType == TypeCategory::VOID)
       return "  " + call_str + ";\n";
-    else if (call.returnType == TypeCategory::INT)
+    else if (call.returnType == TypeCategory::INT ||
+          call.returnType == TypeCategory::FLOAT)
+    {
       return "  var result = " + call_str + ";\n";
+    }
+    else if (call.returnType == TypeCategory::INT64)
+    {
+      // Emscripten saves the high bits of a 64-bit return value in a special
+      // variable called tempRet0. We cannot use bit operators to combine the
+      // values because JavaScript operates on 32-bit integers.

[sergei, 2017/04/12 12:04:32]

I would replace JavaScript by something more specific, like "bitwise and shift
operators".

[Wladimir Palant, 2017/04/13 13:04:40]

This comment is gone - I found that Emscripten exposes a function to combine two
i64 parts, doing exactly the same thing that I have here. So we don't need to
reinvent the wheel. Sadly, the reverse transformation is only used internally
and not exposed as a function.

+      return "  var result = (" + call_str + " >>> 0)" +
+             "             + (Runtime.getTempRet0() >>> 0) * 0x100000000;\n";
+    }
     else if (call.returnType == TypeCategory::DEPENDENT_STRING ||
         call.returnType == TypeCategory::OWNED_STRING)
     {
       std::string result;
       result += "  var string = createString();\n";
       result += "  " + call_str + ";\n";
       result += "  var result = readString(string);\n";
       if (call.returnType == TypeCategory::OWNED_STRING)
         result += "  Module._DestroyString(string);\n";
       return result;
@@ skipping 43 matching lines @@
       std::string argName("arg" + std::to_string(i));
       if (i > 0)
         prefix += ", ";
       prefix += argName;
 
       if (call.args[i] == TypeCategory::STRING_REF)
       {
         hasStringArgs = true;
         params.push_back(std::string("createString(") + argName + ")");
       }
+      else if (call.args[i] == TypeCategory::CLASS_PTR)
+        params.push_back(argName + "._pointer");
+      else if (call.args[i] == TypeCategory::INT64)
+      {
+        // 64-bit integers are passed as two integer parameters
+        params.push_back(argName + " >>> 0");
+        params.push_back(argName + " / 0x100000000 >>> 0");

[sergei, 2017/04/12 12:04:32]

It would be very useful to have few tests for that because it can work for some
values but break for bigger or smaller values which are not common and difficult
to catch.

[Wladimir Palant, 2017/04/13 13:04:40]

We have a few tests in FilterNotifier further up my patch queue but I've added
better values to subscriptionClasses tests as well now.

+      }
       else
         params.push_back(argName);
     }
     prefix += ")\n{\n";
 
     std::string suffix = "}";
     if (call.returnType != TypeCategory::VOID)
       suffix = "  return result;\n" + suffix;
 
     if (call.returnType == TypeCategory::DEPENDENT_STRING ||
@@ skipping 258 matching lines @@
 
     std::vector<std::pair<int, std::string>> mapping;
     for (const auto& item : list)
       mapping.emplace_back(item.first, item.second);
 
     bindings_internal::register_differentiator(
         bindings_internal::TypeInfo<ClassType>(), offset, mapping);
     return *this;
   }
 };