Noissue - [emscripten] Call the correct constructor for strings created from JavaScript

compiled/bindings/generator.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
- * Copyright (C) 2006-2017 eyeo GmbH
+ * 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
@@ skipping 206 matching lines @@
       case TypeCategory::INT64:
         return "  var result = Runtime.makeBigInt(" + call_str + ", " +
                                                   "Runtime.getTempRet0(), " +
                                                   "true);\n";
       case TypeCategory::DEPENDENT_STRING:
       {
         std::string result;
         result += "  var string = createString();\n";
         result += "  " + call_str + ";\n";
         result += "  var result = readString(string);\n";
+        // We don't call a destructor here because we know that dependent
+        // strings don't need to clean up.
         return result;

[sergei, 2017/08/23 10:06:15]

Strictly speaking, despite destructor of DependentString does nothing it is
still should be called or we should put a comment here that it's an
optimization. It seems another issue, though, but can be done here.

[Wladimir Palant, 2017/08/23 10:26:41]

Ok, I added a comment. In fact, we might need to call the destructor in debug
mode if we add reference counting.

[sergei, 2017/08/23 10:39:19]

Yeah, I thought about similar debug needs too. If it's possible we could maybe
have that call optional. The current variant is fine for me, it can be added
when there is a real demand for that.

       }
       case TypeCategory::OWNED_STRING:
       {
         std::string result;
         result += "  var string = createOwnedString();\n";
         result += "  " + call_str + ";\n";
         result += "  var result = readString(string);\n";

[sergei, 2017/08/23 10:06:15]

Since anyway we have separate calls of Module._InitOwnedString(result); and
Module._DestroyString(string); we could move the latter into a readString which
calls Module._DestroyString(string);. It could save few bytes of JS code.

[Wladimir Palant, 2017/08/23 10:26:41]

I tried that but it makes the code here less obvious IMHO. The few bytes saved
don't really matter either.

[sergei, 2017/08/23 10:39:19]

Acknowledged.

         result += "  Module._DestroyString(string);\n";
         return result;
       }
       case TypeCategory::STRING_REF:
         return "  var result = readString(" + call_str + ");\n";
       case TypeCategory::CLASS_PTR:
       {
         std::string result;
         result += "  var result = " + call_str + ";\n";
         result += "  if (result)\n";
@@ skipping 211 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);
 }