Code Improvements - Removing Macros

[stephenberry]

Just found this scripting library and it looks like a fantastic start. I had started writing my own scripting language with the same intention, but I'd rather just contribute to this one. Using a subset of C++11 for the scripting language is just wonderful!

After digging through some of the code and testing I have a number of recommendations and questions.

First, can C++17 can be used as the compilation language? There are so many benefits to using 17 over 11 when it comes to writing this library and making it performant. I'll explain more as I address individual concerns. But, C++17 support is pretty robust and easily available now and you'll be able to make your library much cleaner, safer, with less macros, and faster running. You could still keep the scripting portion to a subset of 11.

I'll focus this issue on macros though.

• I highly recommend moving away from macros
  • I understand the desire for clean registration, but it makes registration code harder to debug, pollutes the global namespace, works against sharing registration function behavior and reducing complexity, makes constexpr optimizations harder to implement, makes error handling of invalid inputs more difficult to implement, creates issues when trying to move to more modern C++ (e.g. modules), and in this case can lead to even more typing and less customization for the user. I could go on.
  • I think it is fine for the user to have to pass in strings names of types where required, but in a lot of cases you don't need the user to provide the type names. For example, the environment can have a std::map<std::type_index, std::string> that maps types to names and can be used during registration to get the appropriate name from a passed in or compile time deduced type. You'll find that you'll actually need less typing than the macros in a lot of cases because the compiler will be able to deduce things like return types and often even input types from a function.

Here is an example of moving away from the macros for ValueAs and RegisterFunctionReturnParams1:

namespace Cflat
{
   template <class Type>
   inline decltype(auto) ValueAs(const Value* pValuePtr)
   {
      return *reinterpret_cast<std::conditional_t<std::is_lvalue_reference_v<Type>, typename std::remove_reference_t<Type>, Type>*>((pValuePtr)->mValueBuffer);
   }

   template <class RetType, class Param0Type>
   inline void RegisterFunctionReturnParams1(Cflat::Environment* pEnv, RetType (*pFunction) (Param0Type), const char* pFunctionName)
   {
      Cflat::Function* function = (pEnv)->registerFunction(pFunctionName);
      function->mReturnTypeUsage = (pEnv)->getTypeUsage((pEnv)->getTypeName<RetType>()); CflatValidateTypeUsage(function->mReturnTypeUsage);
      function->mParameters.push_back((pEnv)->getTypeUsage((pEnv)->getTypeName<Param0Type>())); CflatValidateTypeUsage(function->mParameters.back());
      function->execute = [function, pFunction](const CflatArgsVector(Cflat::Value)& pArguments, Cflat::Value* pOutReturnValue)
      {
         CflatAssert(function->mParameters.size() == pArguments.size());
         CflatAssert(pOutReturnValue);
         CflatAssert(pOutReturnValue->mTypeUsage.compatibleWith(function->mReturnTypeUsage));
         auto result = pFunction(ValueAs<Param0Type>(&pArguments[0]));
         pOutReturnValue->set(&result);
      };
   }
}

The macro version registration was: CflatRegisterFunctionReturnParams1(&env, int, func, int);

This non macro version looks like: Cflat::RegisterFunctionReturnParams1(&env, func, "func") Or, if func is overloaded we have to specify the input and return types: Cflat::RegisterFunctionReturnParams1<int, int>(&env, func, "func")

So, there isn't much more typing for the non macro version. It gives the user the ability to specify the name in the scripting engine, and it brings all the benefits of moving away from macros.

I am happy to help move away from macros. I just want to get your thoughts before starting to submit pull requests.

[arturocepeda]

Hi Stephen!

First of all, I'm glad you like the project and really appreciate your desire to contribute, which is most certainly welcome!

Regarding the switch to C++17, I honestly would prefer to wait until I find a really strong reason to do it. The original idea was to make Cflat usable for as many compilers and project configurations as possible out there, and, while it's true that more and more projects support C++17 or higher these days, in general the lower the required C++ version, the less likely it is that Cflat gets discarded as a potential solution for a project where it might be helpful - I know for a fact that many C++ programmers prefer to stay away from the modern standards.

I have taken a quick look at all the issues you have open before starting answering here, and therefore I know that the first reason (surely not the only one) why you would like to have C++17 support is std::string_view, which makes sense, since, as you point out, that would allows us to save the calls to strlen. I actually think it's a good idea to get rid of those strlen calls, but, at least as a first approach, I would rather just add a member to Identifier which stores the string size.

As for the macros, I totally agree: they could be easily replaced with inline functions without any real disadvantages, aside from the need to pass the function name in the case the ones to register functions, which would definitely not be a big deal. Having said that, the strongest reason I currently see to keep the macros, at least for the time being, are the existing ongoing projects that make use of them (I'm involved in two at the moment, one of them being a big one, but there might be more I don't know about yet). I wouldn't mind having the inline functions as an additional option, though.

One thing I would like to do at some point, but haven't really given a thought to yet, is getting rid of the "Params" variations and instead provide templates with a variable number of arguments. I guess that isn't doable with C++11, and the possibility of having it would make me seriously consider switching to a more modern standard, but I think even in that case I would probably still prefer to make it configurable and only provide the templates optionally for people willing to use the newer version of C++.

I will now go through the other issues and the pull request commits!

Thank you!