Code is not complilable

JBenda / inkcpp

Inkle Ink C++ Runtime with JSON>Binary Compiler

MIT License

73 stars 15 forks source link

Code is not complilable #5

Closed Nordsoft91 closed 3 years ago

Nordsoft91 commented 3 years ago

Looks like code was tested only under Win environment (MSVS)

Compiling with AppleClang 9.1.0 shows multiple compile-time errors.

Please, build target under *NIX environment

brwarner commented 3 years ago

Hey Nordsoft,

Thanks for reporting this. I'll give it a shot on Linux when I next put some time into this.

Just to check, are you compiling with C++17 features enabled on your compiler? I setup CMake to enable it but I'm not sure it works on all environments.

2shady4u commented 3 years ago

@brwarner Just tested out Github Actions for Windows, Linux and MacOS.

https://github.com/2shady4u/inkcpp/actions

Seems there's still some work to make the code truly platform-agnostic.

PS: This fork uses the scons build system instead of cmake to generate platform-dependent build files. Unfortunately I don't have the proper knowledge/experience about cmake to implement this in Github Actions... 🤔

Nordsoft91 commented 3 years ago

@brwarner, yes, C++17 used. Quick review has shown:

There are some platform-specific functions you use, e.g. _itoa_s.
Template specialization should be out of class definition
std::exception doesn't receive std::string in ctor, better to use std::runtime_error or std::logic_error (or create own expectation inherits std::exception)

brwarner commented 3 years ago

Thanks. I'm part way there in the new platform branch. There's some sticky template shenanigans that I'm having trouble parsing but I'll take a look at them next time.

brwarner commented 3 years ago

With some help from JBrenda this should now compile on Linux. There may be other issues, which I'll try to look into.