Issue 5137 - [emscripten] Added basic filter storage implementation

compiled/bindings/generator.cpp

 /*
  * 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 #include <cstdio>
 
 #include <emscripten.h>
 
 #include "generator.h"
 
 namespace
 {
   std::vector<bindings_internal::ClassInfo> classes;
+  std::vector<bindings_internal::NamespaceInfo> namespaces;
+
+  void printProperties(const bindings_internal::PropertyList& properties)
+  {
+    for (const auto& property : properties)
+    {
+      if (property.jsValue.empty())
+      {
+        printf("get %s%s,\n", property.name.c_str(),
+               wrapCall(property.getter, false).c_str());
+        if (!property.setter.empty())
+        {
+          printf("set %s%s,\n", property.name.c_str(),
+                 wrapCall(property.setter, false).c_str());
+        }
+      }
+      else
+        printf("%s: %s,\n", property.name.c_str(), property.jsValue.c_str());
+    }
+  }
+
+  void printMethods(const bindings_internal::MethodList& methods,
+                    bool instanceOnly = true)
+  {
+    for (const auto& method : methods)
+      if (!instanceOnly || method.call.instance_function)
+        printf("%s: %s,\n", method.name.c_str(), wrapCall(method.call).c_str());
+  }
 }
 
 namespace bindings_internal
 {
   FunctionInfo::FunctionInfo()
   {
     name[0] = '\0';
   }
 
   FunctionInfo::FunctionInfo(TypeCategory returnType, TYPEID pointerType,
@@ skipping 211 matching lines @@
 
     if (classInfo->subclass_differentiator.offset != SIZE_MAX)
       throw std::runtime_error("More than one subclass differentiator defined for class " + classInfo->name);
 
     DifferentiatorInfo differentiatorInfo;
     differentiatorInfo.offset = offset;
     differentiatorInfo.mapping = mapping;
     classInfo->subclass_differentiator = differentiatorInfo;
   }
 
+  NamespaceInfo* find_namespace(const char* namespaceName)
+  {
+    for (auto& namespaceInfo : namespaces)
+      if (namespaceInfo.name == namespaceName)
+        return &namespaceInfo;
+    return nullptr;
+  }
+
+  void register_namespace(const char* name)
+  {
+    if (find_namespace(name))
+      throw std::runtime_error(std::string("Duplicate definition for namespace ") + name);
+
+    NamespaceInfo namespaceInfo;
+    namespaceInfo.name = name;
+    namespaces.push_back(namespaceInfo);
+  }
+
+  void register_namespace_property(const char* namespaceName, const char* name,
+      const FunctionInfo& getter, const FunctionInfo& setter)
+  {
+    NamespaceInfo* namespaceInfo = find_namespace(namespaceName);
+    if (!namespaceInfo)
+      throw std::runtime_error(std::string("Property defined on unknown namespace: ") + name);
+
+    PropertyInfo propertyInfo;
+    propertyInfo.name = name;
+    propertyInfo.getter = getter;
+    propertyInfo.setter = setter;
+    namespaceInfo->properties.push_back(propertyInfo);
+  }
+
+  void register_namespace_method(const char* namespaceName, const char* name,
+      const FunctionInfo& call)
+  {
+    NamespaceInfo* namespaceInfo = find_namespace(namespaceName);
+    if (!namespaceInfo)
+      throw std::runtime_error(std::string("Method defined on unknown namespace: ") + name);
+
+    MethodInfo methodInfo;
+    methodInfo.name = name;
+    methodInfo.call = call;
+    namespaceInfo->methods.push_back(methodInfo);
+  }

[Wladimir Palant, 2017/05/01 14:47:25]

I'm not really happy with the complexity required to expose namespaces. We
should probably discuss changes to the bindings definition API - maybe the code
can get considerably simpler if we go away from the embind-style APIs.

[sergei, 2017/05/08 10:54:39]

I would like to include in that discussion the design, in particular global
variables like knownFilters, FilterNotifier and subscriptions (in
FilterStorage.cpp), not loosely coupling like Listed property of subscription,
and global things mentioned above like FilterNotifier and in some other places.
So far, I find these things OK but only because it closely repeats the original
JS version, so the number of bugs should be less, but later it would be better
to reconsider such things.

In particular here, FilterStorage should be a class and we will get rid of the
namespace. However I would like to have the whole library in a namespace, if
it's a bit tricky then we could use for instance ABP prefix for everything.

[Wladimir Palant, 2017/05/08 12:55:41]

I considered making FilterStorage a singleton class. The downside here is that
calling instance methods from JavaScript is considerably less efficient while
not offering any benefit here whatsoever.

[sergei, 2017/08/24 13:32:04]

Singleton and global variables are actually equally bad from design point of
view. I would wait for finishing of the transformation, then take a look what
and how is actually used and then restructure in in way to get rid of these
global things without hurting the performance.

+
   std::string generateCall(const FunctionInfo& call,
       std::vector<std::string>& params)
   {
     if (call.returnType == TypeCategory::DEPENDENT_STRING ||
         call.returnType == TypeCategory::OWNED_STRING)
     {
       params.insert(params.begin(), "string");
     }
 
     std::string call_str(call.name);
@@ skipping 73 matching lines @@
       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");
+        params.push_back(argName + " ? " + argName + "._pointer : 0");

[Wladimir Palant, 2017/05/01 14:47:25]

Turns out we weren't supporting null for parameters expecting a class.

[sergei, 2017/05/08 10:54:39]

In addition I wonder whether we can use C++ references, we do use them for
strings already, why not to use them also for other objects.

[Wladimir Palant, 2017/05/08 12:55:41]

We could use references easily but these won't be references in JavaScript. The
bindings code will then have to make sure that no null pointers are passed in.
So far this didn't seem to be worth doing but we can certainly consider it.

[sergei, 2017/08/24 13:32:04]

Currently one has to manually test whether a pointer argument is null or not
each time in a method implementation, however many functions expect a real
object and this testing is just additional code which can be avoided. Therefore
using C++ references is safer and easier in many cases. However, if we
accidentally pass not a "real object" from JS into a function which does not
expect it, it would be better to generate the check into bindings than manually
implement it in a method implementation.

In addition I guess we should think about it a little bit more because it can
help to develop some other conventions, e.g. ref_counted objects should be
always allocated on heap and never on the stack.

[Wladimir Palant, 2017/08/31 11:32:34]

I filed https://issues.adblockplus.org/ticket/5603 on that.

       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");
       }
       else
         params.push_back(argName);
     }
     prefix += ")\n{\n";
@@ skipping 63 matching lines @@
           Module._ReleaseRef(this._pointer + ref_counted_offset);
         };
         return result;
       }
     )");
   }
 
   void printClass(const ClassInfo& cls)
   {
     // Begin class definition
-
     ClassInfo* baseClass = find_class(cls.baseClass);
     printf("exports.%s = createClass(%s, %i, {\n", cls.name.c_str(),
         (baseClass ? ("exports." + baseClass->name).c_str() : "null"),
         cls.ref_counted_offset);
 
     // Print prototype members
-
-    for (const auto& property : cls.properties)
-    {
-      if (property.jsValue.empty())
-      {
-        printf("get %s%s,\n", property.name.c_str(),
-               wrapCall(property.getter, false).c_str());
-        if (!property.setter.empty())
-        {
-          printf("set %s%s,\n", property.name.c_str(),
-                 wrapCall(property.setter, false).c_str());
-        }
-      }
-      else
-        printf("%s: %s,\n", property.name.c_str(), property.jsValue.c_str());
-    }
-
-    for (const auto& method : cls.methods)
-      if (method.call.instance_function)
-        printf("%s: %s,\n", method.name.c_str(), wrapCall(method.call).c_str());
+    printProperties(cls.properties);
+    printMethods(cls.methods);
 
     // End class definition
-
     printf("});\n");
 
     // Print static members
-
     DifferentiatorInfo differentiator = cls.subclass_differentiator;
     if (differentiator.offset != SIZE_MAX)
     {
       printf("var %s_mapping = \n", cls.name.c_str());
       puts("{");
       for (const auto& item : differentiator.mapping)
         printf("  %i: '%s',\n", item.first, item.second.c_str());
       puts("};");
 
       printf("exports.%s.fromPointer = function(ptr)\n", cls.name.c_str());
@@ skipping 14 matching lines @@
 
     for (const auto& method : cls.methods)
     {
       if (!method.call.instance_function)
       {
         printf("exports.%s.%s = %s;\n", cls.name.c_str(), method.name.c_str(),
                wrapCall(method.call).c_str());
       }
     }
   }
+
+  void printNamespace(const NamespaceInfo& namespaceInfo)
+  {
+    printf("exports.%s = {\n", namespaceInfo.name.c_str());
+    printProperties(namespaceInfo.properties);
+    printMethods(namespaceInfo.methods, false);
+    puts("};");
+  }
 }
 
 void printBindings()
 {
   bindings_internal::printHelpers();
 
   for (const auto& item : classes)
     bindings_internal::printClass(item);
+  for (const auto& item : namespaces)
+    bindings_internal::printNamespace(item);
 }