Platform #6
Platform #6
Conversation
- `static_assert(false,...);` can't be used because compiler is smart
use `static_assert(!std::is_same(T,T)::value, ...);` instead
- explicit declare base class calls !
JBenda
left a comment
JBenda
left a comment
There was a problem hiding this comment.
Just some commands to describe the work.
| fixed_restorable_array(const T& initial, const T &nullValue) : basic_restorable_array<T>(_buffer, SIZE * 2, nullValue) | ||
| { | ||
| clear(initial); | ||
| basic_restorable_array<T>::clear(initial); |
There was a problem hiding this comment.
Call function from base explicit to avoid compiler misunderstandings.
|
|
||
| template<typename T> | ||
| static void function_base::push(basic_eval_stack* stack, const T& value) | ||
| void function_base::push(basic_eval_stack* stack, const T& value) |
There was a problem hiding this comment.
Static qualifier for function definition outside of class is forbidden (also is not part of the signature so ...).
| // pops an argument from the stack using the function-type | ||
| template<int index> | ||
| typename arg_type<index> pop_arg(basic_eval_stack* stack) | ||
| arg_type<index> pop_arg(basic_eval_stack* stack) |
There was a problem hiding this comment.
typename can only be for going into something (like foo<int>::bar), in this case it can only be a type, so it's forbidden to write it.
|
|
||
| // void functions | ||
| if constexpr (is_same<void, traits::return_type>::value) | ||
| if constexpr (is_same<void, typename traits::return_type>::value) |
There was a problem hiding this comment.
The compiler doesn't know that return_type in traits is a type, so we need the typename qualifier.
| public: | ||
| // No public interface yet | ||
| virtual void dummy() = 0; | ||
| virtual ~globals_interface() = default; |
There was a problem hiding this comment.
A virtual class should have a virtual deconstruct.
| if (p.path().extension() == ".ink") | ||
| { | ||
| bool success = test(p.path().u8string()); | ||
| bool success = test(p.path().string()); |
There was a problem hiding this comment.
The declaration of test wants a std::string so we give it :)
| #include "config.h" | ||
| #ifdef INK_EXPOSE_JSON | ||
| #include "json.hpp" | ||
| #include "../json.hpp" |
There was a problem hiding this comment.
In Linux the compiler is quite precise when you use a relative path #include "header", the json.hpp is one directory about compiler.h.
| // Make sure our buffer is empty | ||
| #ifdef WIN32 | ||
| _Tidy(); | ||
| #endif |
There was a problem hiding this comment.
Tidy is a MS VCS thing, so no support for Linux.
|
|
||
| #ifdef INK_COMPILER | ||
| const char* CommandStrings[]; | ||
| extern const char* CommandStrings[]; |
There was a problem hiding this comment.
Write external to only declare it and not define it.
|
|
||
| #ifdef INK_ENABLE_STL | ||
| using ink_exception = std::exception; | ||
| using ink_exception = std::runtime_error; |
There was a problem hiding this comment.
std::runtime_error is more precise for this use case and can be constructed from string
|
Hey! This is really helpful @JBenda. Thanks so much. The requirement to sometimes add the typename/template keyword always gets me confused. I wish the MSVC compiler would just match requirements of the C++ standard so I wouldn't have accumulated so many issues. Your fixes are great; I've merged them into the branch. If everything compiles fine on my machine I'll then compile it into master. If you want to continue to contribute, shoot me an email at [email protected] and maybe I can work out some tasks that need doing. Otherwise my next goal is to get the ink-proof testing system running on this project and then just one by one fixing the broken tests (so far it looks like I'm failing 80% of them lol). |
2 participants
For Issue #5
Hey I really like the project, so I thought maybe let it compile for GCC (and Linux) is a nice warm up.
When my coding style doesn't fit your convention I will change it happily. Also, I think there is maybe one or two things you would to discuss.
A minimal example runs, also inkcpp_test runs without complaints (it is green).