Noissue - fix generation of a new instance creation of a class without subclass differentiator

compiled/bindings/generator.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 239 matching lines @@
       case TypeCategory::CLASS_REF:
       {
         std::string result;
         result += "  var result = " + call_str + ";\n";
         result += "  if (result)\n";
 
         const ClassInfo* cls = find_class(call.pointerType);
         if (!cls)
           throw std::runtime_error("Function " + call.name + " returns pointer to unknown class");
 
-        result += "    result = exports." + cls->name + ".fromPointer(result);\n";

[sergei, 2017/09/13 17:09:10]

Another option here could be to keep the if and for the branch without the
differentiator put ("new " added):
`result += "    result = new exports." + cls->name + "(result);\n";`
and remove the generation of implementation of fromPointer for such classes.
However, I think it can be convenient for debugging and maybe even for some
future changes to call constructors only from "fromPointer".
What is your opinion?

[Wladimir Palant, 2017/09/19 08:51:48]

Trouble is, bindings overhead is very much non-negligible - that's the reason
why I am micro-optimizing here. So I'd rather keep the overhead as low as
possible and not call fromPointer unless necessary. In fact, fromPointer was
inlined here originally and I am still not sure whether turning it into a method
call was the right decision to make.

[sergei, 2017/09/19 09:14:35]

I have uploaded the version without fromPointer for classes without the subclass
differentiator.

+        auto offset = cls->subclass_differentiator.offset;
+        if (offset == SIZE_MAX)
+          result += "    result = new exports." + cls->name + "(result);\n";
+        else
+          result += "    result = exports." + cls->name + ".fromPointer(result);\n";
 
         result += "  else\n";
         result += "    result = null;\n";
         return result;
       }
       default:
         throw std::runtime_error("Unexpected return type for " + call.name);
     }
   }
 
@@ skipping 167 matching lines @@
     if (differentiator.offset != SIZE_MAX)
     {
       printf("exports.%s.fromPointer = function(ptr)\n", cls.name.c_str());
       puts("{");
       printf("  var type = HEAP32[ptr + %i >> 2];\n", differentiator.offset);
       printf("  if (type in %s_mapping)\n", cls.name.c_str());
       printf("    return new %s_mapping[type](ptr);\n", cls.name.c_str());
       printf("  throw new Error('Unexpected %s type: ' + type);\n", cls.name.c_str());
       puts("};");
     }
-    else
-    {
-      printf("exports.%s.fromPointer = function(ptr)\n", cls.name.c_str());
-      puts("{");
-      printf("  return new exports.%s(ptr);\n", cls.name.c_str());
-      puts("};");
-    }
 
     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());
       }
     }
   }
@@ skipping 14 matching lines @@
 
 void printBindings()
 {
   bindings_internal::printHelpers();
 
   for (const auto& cls : classes)
     bindings_internal::printClass(cls);
   for (const auto& cls : classes)
     bindings_internal::printClassMapping(cls);
 }