Remove unnecessary restrictions around RTTI from wire.h (#10914)

Fixes #6551

Removes the static_asserts that check if the type for a LightTypeID is_polymorphic.
AFAICT, LightTypeID works fine with polymorphic types, and provides the same
identity guarantees as when RTTI is enabled. Adds a test asserting such code
compiles and runs correctly.

I'm still not clear why this restriction was in place - it may have been necessary
in previous impelmentations, or perhaps with previous versions of LLVM/Clang.
However, as far as I can tell, it's not necessary now and only limits options
around doing non-RTTI builds with emscripten. Attempting to create a testcase
showing why the restrictions makes sense fails (see discussion in PR).

AUTHORS

@@ -464,6 +464,7 @@ a license to everyone to use it as detailed in LICENSE.)
 * Michael Potthoff <michael@potthoff.eu>
 * Abigail Bunyan <abigail@bold.claims> (copyright owned by Microsoft Corporation)
 * David García Paredes <davidgparedes@gmail.com>
+* Dan Field <dfield@gmail.com> (copyright owned by Google, Inc.)
 * Mike Swierczek <mike@swierczek.io>
 * Vasily <just.one.man@yandex.ru>
 * Jānis Rūcis <parasti@gmail.com>

ChangeLog.md

@@ -17,6 +17,7 @@ See docs/process.md for how version tagging works.
 
 Current Trunk
 -------------
+- Allow polymorphic types to be used without RTTI when using embind. (#10914)
 - Only strip the LLVM producer's section in release builds. In `-O0` builds, we
   try to leave the wasm from LLVM unmodified as much as possible, so if it
   emitted the producers section, it will be there. Normally that only matters
@@ -448,7 +449,7 @@ v1.39.5: 12/20/2019
 - Default `DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR` to 1. See #9895.
   With this change the old deprecated HTML5 API event target lookup behavior is
   disabled. There is no "Module.canvas" object, no magic "null" default handling,
-  and DOM element 'target' parameters are taken to refer to CSS selectors, instead 
+  and DOM element 'target' parameters are taken to refer to CSS selectors, instead
   of referring to DOM IDs. For more information see:
   <https://groups.google.com/forum/#!msg/emscripten-discuss/xScZ_LRIByk/_gEy67utDgAJ>
 - WASI API updated from `wasi_unstable` to `wasi_snapshot_preview1`. This
@@ -1915,7 +1916,7 @@ v1.34.5: 8/18/2015
  - Fixed issues with building the optimizer on 32-bit Windows (#3673)
  - Increased optimizer stack size on Windows to 10MB (#3679)
  - Added support for passing multiple input files to opt, to speed up
-   optimization and linking in opt.  
+   optimization and linking in opt.
  - Full list of changes:
     - Emscripten: https://github.com/emscripten-core/emscripten/compare/1.34.4...1.34.5
     - Emscripten-LLVM: https://github.com/emscripten-core/emscripten-fastcomp/compare/1.34.4...1.34.5
@@ -2399,7 +2400,7 @@ v1.29.2: 1/16/2015
  - Fixed an issue with SDL audio queueing stability, which would queue audio too
    eagerly and cause stutter in some applications (#3122, #3124)
  - Enabled native JS optimizer to be built automatically on Windows, requires
-   VS2012 or VS2013. 
+   VS2012 or VS2013.
  - Improve error message to reflect the fact that DLOPEN_SUPPORT is currently
    not available (#2365)
  - Improve SIMD load and store support.
@@ -2732,7 +2733,7 @@ v1.23.1: 8/26/2014
  - Removed the support for using Web Audio ScriptProcessorNode to stream audio.
  - Improved SDL audio streaming by using the main rAF() callback instead of a
    separate setTimeout() callback to schedule the audio data.
- - Deprecated compiling without typed arrays support. 
+ - Deprecated compiling without typed arrays support.
  - Migrated to using musl PRNG functions. Fixes reported bugs about the quality of randomness (#2341)
  - Improved SIMD support for the experimental Ecmascript SIMD spec.
  - Full list of changes:
@@ -3210,7 +3211,7 @@ v1.17.0: 5/6/2014
 
 v1.16.0: 4/16/2014
 ------------------
- - Removed browser warnings message in VFS library about replacing __proto__ performance issue. 
+ - Removed browser warnings message in VFS library about replacing __proto__ performance issue.
  - Full list of changes:
     - Emscripten: https://github.com/emscripten-core/emscripten/compare/1.15.1...1.16.0
     - Emscripten-LLVM: no changes.
@@ -3221,7 +3222,7 @@ v1.15.1: 4/15/2014
  - Added support for SDL2 touch api.
  - Added new user-controllable emdind-related define #define
    EMSCRIPTEN_HAS_UNBOUND_TYPE_NAMES, which allows optimizing embind for minimal
-   size when std::type_info is not needed. 
+   size when std::type_info is not needed.
  - Fixed issues with CMake support where CMAKE_AR and CMAKE_RANLIB were not
    accessible from CMakeLists.txt files.
  - Full list of changes:
@@ -3418,7 +3419,7 @@ v1.11.0: 2/14/2014
 v1.10.4: 2/10/2014
 ------------------
  - Added support for legacy GL emulation in fastcomp.
- - Deprecated the --split-js compiler option. This is not supported in fastcomp. 
+ - Deprecated the --split-js compiler option. This is not supported in fastcomp.
  - Full list of changes: https://github.com/emscripten-core/emscripten/compare/1.10.3...1.10.4
 
 v1.10.3: 2/9/2014
@@ -3698,7 +3699,7 @@ v1.7.3: 11/12/2013
    lose precision in the function call.
  - Added support for joysticks in SDL via the Gamepad API
  - Full list of changes: https://github.com/emscripten-core/emscripten/compare/1.7.2...1.7.3
- 
+
 v1.7.2: 11/9/2013
 ------------------
  - The compiler now always generates a .js file that contains the generated
@@ -3713,7 +3714,7 @@ v1.7.2: 11/9/2013
  - Added a new command line parameter --no-heap-copy to compiler and file
    packager that can be used to optimize VFS memory usage at startup.
  - Updated libcxx to revision 194185, 2013-11-07.
- - Improvements to various library support. 
+ - Improvements to various library support.
  - Full list of changes: https://github.com/emscripten-core/emscripten/compare/1.7.1...1.7.2
 
 v1.7.1: 10/24/2013
@@ -3810,7 +3811,7 @@ v1.6.0: 9/21/2013
  - Enable support for %[] pattern in scanf.
  - Added dependency tracking support to linked .js files in CMake toolchain.
  - The hex prefix 0x is now properly handled in sscanf (#1632).
- - Simplify internal compiler operations by removing the internal framework.js. 
+ - Simplify internal compiler operations by removing the internal framework.js.
  - Full list of changes: https://github.com/emscripten-core/emscripten/compare/1.5.9...1.6.0
 
 v1.5.9: 9/15/2013
@@ -4035,7 +4036,7 @@ v1.3.3: 3/23/2013
 
 v1.3.2: 3/22/2013
 ------------------
- - Fix issues with fgets. 
+ - Fix issues with fgets.
  - Add support for non-fullscreen pointer lock.
  - Improve OpenAL support.
  - Full list of changes: https://github.com/emscripten-core/emscripten/compare/1.3.1...1.3.2

system/include/emscripten/wire.h

@@ -64,7 +64,7 @@ namespace emscripten {
         struct LightTypeID {
             static constexpr TYPEID get() {
                 typedef typename Canonicalized<T>::type C;
-                if(has_unbound_type_names || std::is_polymorphic<C>::value) {
+                if(has_unbound_type_names) {
 #if __has_feature(cxx_rtti)
                     return &typeid(T);
 #else
@@ -72,8 +72,6 @@ namespace emscripten {
                         "Unbound type names are illegal with RTTI disabled. "
                         "Either add -DEMSCRIPTEN_HAS_UNBOUND_TYPE_NAMES=0 to or remove -fno-rtti "
                         "from the compiler arguments");
-                    static_assert(!std::is_polymorphic<C>::value,
-                        "Canonicalized<T>::type being polymorphic is illegal with RTTI disabled");
 #endif
                 }
 
@@ -84,7 +82,7 @@ namespace emscripten {
         template<typename T>
         constexpr TYPEID getLightTypeID(const T& value) {
             typedef typename Canonicalized<T>::type C;
-            if(has_unbound_type_names || std::is_polymorphic<C>::value) {
+            if(has_unbound_type_names) {
 #if __has_feature(cxx_rtti)
                 return &typeid(value);
 #else
@@ -92,8 +90,6 @@ namespace emscripten {
                     "Unbound type names are illegal with RTTI disabled. "
                     "Either add -DEMSCRIPTEN_HAS_UNBOUND_TYPE_NAMES=0 to or remove -fno-rtti "
                     "from the compiler arguments");
-                static_assert(!std::is_polymorphic<C>::value,
-                    "Canonicalized<T>::type being polymorphic is illegal with RTTI disabled");
 #endif
             }
             return LightTypeID<T>::get();

tests/core/test_embind_polymorphic_class_no_rtti.cpp

@@ -0,0 +1,42 @@
+// Copyright 2020 The Emscripten Authors.  All rights reserved.
+// Emscripten is available under two separate licenses, the MIT license and the
+// University of Illinois/NCSA Open Source License.  Both these licenses can be
+// found in the LICENSE file.
+
+#include <emscripten.h>
+#include <emscripten/bind.h>
+#include <emscripten/val.h>
+#include <stdio.h>
+
+EM_JS(void, calltest, (), {
+  var foo = new Module.Foo();
+  console.log("foo.test() returned: " + foo.test());
+});
+
+class Foo {
+  public:
+  virtual ~Foo() = default;
+  virtual int test() = 0;
+};
+
+class Bar : public Foo {
+  public:
+  int test() override { return 42; }
+};
+
+int main(int argc, char** argv){
+  printf("Hello, world.\n");
+  calltest();
+  return 0;
+}
+
+std::shared_ptr<Foo> MakeFoo() {
+  return std::make_shared<Bar>();
+}
+
+EMSCRIPTEN_BINDINGS(my_module) {
+  emscripten::class_<Foo>("Foo")
+    .smart_ptr_constructor("Foo", &MakeFoo)
+    .function("test", &Foo::test)
+    ;
+};

tests/core/test_embind_polymorphic_class_no_rtti.out

@@ -0,0 +1,2 @@
+Hello, world.
+foo.test() returned: 42

tests/test_core.py

@@ -6988,6 +6988,10 @@ someweirdtext
     self.emcc_args += ['--bind', '-fno-rtti', '-DEMSCRIPTEN_HAS_UNBOUND_TYPE_NAMES=0']
     self.do_run(src, '418\ndotest returned: 42\n')
 
+  def test_embind_polymorphic_class_no_rtti(self):
+    self.emcc_args += ['--bind', '-fno-rtti', '-DEMSCRIPTEN_HAS_UNBOUND_TYPE_NAMES=0']
+    self.do_run_in_out_file_test('tests', 'core', 'test_embind_polymorphic_class_no_rtti.cpp')
+
   def test_embind_no_rtti_followed_by_rtti(self):
     src = r'''
       #include <emscripten.h>