Support compiling Happly without RTTI.

[jdumas]

I needed to compile Happly for a wasm project that wants to be compiled without RTTI, so this PR allows supporting this configuration. It comes with a custom RTTI implementation based on LLVM-style RTTI, but because they have some limitations compared to compiled-generated RTTIs (mostly when loaded across DLL boundaries), this alternative is disabled by default (and only used when RTTI are not available).

[nmwsharp]

Hi! This seems very cool!

I've never dealt with this before, so just to check my understanding: some compilers don't support RTTI, which is used under-the-hood e.g. when calling dyanmic_cast<>, as happly currently does. This blocks you from building happly on such systems. This PR adds (a) a way to detect whether or not the compiler supports RTTI, and (b) some alternate #define'd away code which manually does something RTTI-like, so the library can build without dynamic_cast<>. Is that correct?

If so, I love the idea of being able to compile on more systems, but I'm a little wary of adding build-time complexity to what should be a simple header-only library, I see a bunch of compiler-specific variables and macros. I'm worried that some day in the future some compiler will choke on this block

#if __clang__ && !__INTEL_COMPILER
    #define HAPPLY_ENABLE_RTTI __has_feature(cxx_rtti)
#elif defined(_CPPRTTI)
    #define HAPPLY_ENABLE_RTTI 1
#else
    #define HAPPLY_ENABLE_RTTI (__GXX_RTTI || __RTTI || __INTEL_RTTI__)
#endif

or it would need to be updated for future compilers, etc. Is this concern valid?

On that note, is there any downside to just always using the alternate dynamic_cast<>-free approach? This is all hidden from the user anyway, so we can just change the implementation to avoid difficult-to-compile bits without compiler-specific defines.

[jdumas]

I think most compilers support RTTI. The use-case I have here is we wanted to compile code to wasm, and disabling RTTI (via the -fno-rtti compile flag) can save about 15% percent on the compiled binary size, which is useful when serving an executable on the web.

The block of code that detects whether RTTI are enabled with the current compiler is taken from OneTBB. You can also look at how the BOOST_NO_RTTI macro is implemented, using the same compiler-specific macros. It should work on all major compilers/platforms, but I can help setup a more comprehensive CI matrix with GitHub Actions if you want (idk if your Travis CI is still functional, I don't see where the jobs are going?). Note that in the worst-case, none of those macros are defined and HAPPLY_ENABLE_RTTI will default to 0, defaulting to the custom RTTI system. It would be pretty hard for the whole thing to not compile at all (identifiers with leading underscore are reserved for compiler use, so unlikely some third-party code will be mucking with those either).

On that note, is there any downside to just always using the alternate dynamic_cast<>-free approach?

Unfortunately yes. It wouldn't work across DLL boundaries (when compiling Happly in a separate dll and loading it via dlopen()). Granted it's a very edge-case, and I'm not even sure GCC's internal support this, but MSVC does.

Anyhow, let me know how you want to proceed :)

[nmwsharp]

Ah, thank you for the explanation!

I see the value of this, but I think it is very important to keep the core library as simple and low-maintenance as possible, and I'm a bit worried about the complexity-add here.

How about we create a no-rtti branch, merge this there, and advertise it in the README for anyone else who has the same need? There shouldn't be much trouble keeping branches in-sync, I don't foresee much change to this lib beyond small bugfixes.

[nmwsharp]

(fyi the CI fixed now, it's updated to use github actions)

[jdumas]

¯\(ツ)/

It's your library, so it's your decision in the end. If you don't foresee any big change in the future then imho keeping everything in the same place will make it easier to maintain than having to backport every bugfix into a separate branch. Apart from the RTTIRoot/RTTIExtends classes hidden behind an ifdef, there's not much more addition to the core library.

I could get rid of the ifdef block around these lines by defining RTTIExtends<> to be the identity when the macro is not enabled though, that would simplify a bit the core library code. Then the other changes in the main lib is really (1) call the downcast() method instead of dynamic_cast<>(), and (2) define the static constexpr ID member variable when RTTI is disabled.

[jdumas]

I've updated the PR to fix conflicts and implemented dummy classes to reduce the number of ifdefs in the code. Let me know if this is less intrusive this way.

[jdumas]

Hi @nmwsharp. Have you had a chance to look at the latest version of this PR? I believe it should be less intrusive now. I'd really prefer to avoid having to maintain my own fork/branch of this project.