-
-
Notifications
You must be signed in to change notification settings - Fork 91
Support for Clang compiler (unwrap variadic arguments), fix code errors thrown by -Wpedantic. #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for Clang compiler (unwrap variadic arguments), fix code errors thrown by -Wpedantic. #558
Conversation
Both clang and gcc pass this directly to linker anyway. Signed-off-by: ninetailedtori <[email protected]>
|
Bump to C++20 needed so that Clang can go with |
|
Clang support is definitely a good idea, thanks! However, regarding C++ 20, I'd personally prefer to stay on C++17 as I'd technically have to document the migration as a breaking change (as those who compile from source may need a different/updated version of their compiler) which I prefer to avoid. Is it possible without updating to C++20? |
|
I'd have to use a macro hack that expands based on count of variadics, I think? Lemme see what I can do ;] |
|
Alright, there's some messy ways we could do it - if we attempt to helper function to trick preproc into believing there's always one arg, which there is but, then it'll throw an error in pedantic on variadic-arguments-omitted, which is a valid warn as there'll be a hanging comma. We can instead, define an overload to this macro, which will be my first attempt to see if that works, and that'll do until we bump to c++20, or we have to force gcc, which is messy and sorta ruins the point of this PR xD I'll attempt the overload, but if that fails then I will rewrite the function to handle variadic unwrapping in c++ native, rather than c preproc, and I'll migrate the code to using that safely instead ( which is technically the best safe solution, just a teeny bit less speed if I don't optimise it right but I'll make sure to optimise it well :] ) |
|
But I've cloned the original c++20 migration as well so we have a backup once we do bump to it, so we can revert to speedy preproc (unless I work out a genius native solution, I'll try my best xD) |
…structor (implicitly defined, unnecessary and destroys the implicit copy operator when compiling with pedantic warnings on newer compilers.
e957ebb to
ae54bf8
Compare
|
Alright, I've force pushed a change for this - replaces the entire commit history with a change to VA_ARGS expansion, as well as remove user-defined copy constr for the implicit copy constr which is exactly the same (improves compatibility with strict and pedantic errors, because it throws an error that you've defined one and it's replaced the user-defined copy operator as well). |
|
🎉 This PR is included in version 2.31.0-beta.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
both gcc and clang pass this flag through to linker, so this is for greater compatibility as well.