-
Notifications
You must be signed in to change notification settings - Fork 245
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
Compilation under Visual Studio 2010 #63
Comments
Thanks for sharing your fixes. If any others are in your situation, I'm sure they will find it very useful. You did a great job of documenting exactly where the changes are needed. That said, I'm not sure it makes sense to support VS 2010. VS had/has a lot of idiosyncrasies back then, and I don't think it's unreasonable to insist that a user of tinyexpr should have at least a C99 compliant compiler. I guess my question to you would be, why are you using a 10 year old compiler? C99 was standardized 20 years ago. Don't you miss the features it has available? I'll leave this issue open for a while in case anyone else wants to comment on it. |
Hi. Thank you for your fast response. It is much appreciated. I hope the proposed changes will be useful to someone else. If not, that will be because we are very very few to still work on VS2010. Even if I disagree about support for VS2010, I do understand your considerations. You must also trace a line and that line is to have at least a C99 compliant compiler. As to why I still use VS2010, it is because I created my setup 10+ years ago. It was the setup that I used for work too. I mostly develop on Windows for Windows. I don't miss the new c++ standards too much. Some c++11 features would be great though. I never took time to upgrade my system to a newer version because my ssd is almost full and licensing fees. I guess it is time to update to a new system now... Since your decision has been made and is unlikely to change, feel free to close the issue when you feel like it. Regards. |
I think supporting C89 would be pretty cool. If the differences are mostly aesthetic like variable declarations on the top of the block, being more compatible would come at no cost. I don't like the macros checking the msc version specifically as it wouldn't help if you're using any other compiler. |
Regarding the macros, I agree that multiple As far as I can remember, when I used to program in c, I had to have all variable declarations at the beginning. This might be a Visual Studio only thing or it might have changed in a "not so recent" standard. |
I suppose it wouldn't be too hard to support it. You did all the work already anyway. There may be others with embedded compilers still on C89 as well. I'm going to think about it. |
Although I fully agree with the reluctance to support MS' idioms (as found in VC++10) when there are clearly defined standards, I have actually encountered a case where C89 support would have been great. No big surprise, it is still coming from Redmont: people would download Visual Studio Express don't have c99 support and only get a screen full of error messages when trying to compile tinyexpr. This makes no sense, I really don't understand MS' strategy and it is very confusing for the users who want to recompile software themselves because the not "Express" versions properly support c99... So because of this illogical choice of Microsoft, supporting compiling with c89 only would still be useful even in 2021. |
Hi.
I apologize if my English is not always clear. English is not my native language and I sometime lack using the proper word.
I plan to integrate an expression parser in one of my projects. I have taken a look at tinyexpr and think that it is a really nice library. I found multiple compilation errors when compiling with Visual Studio 2010. I do realize vs2010 is old but this is my development environment for my project. I would be really happy if you would consider supporting vs2010.
When compiling revision ffb0d41, the following errors were identified by Visual Studio:
(I only added one example of each error)
The full error log can be found here: Compilation of ffb0d41b13e5f8d318db95feb071c220c134fe70 under VS2010.txt
Most of these errors are not critical and could be easily resolved by adding the declarations of variables at the beginning of the functions instead of after
if
statements. I hope you are open to suggestion and that you would like to support this compiler. I also wish that you would not count this change "as a new feature" and that it does not break your contributing rules and keep tinyexpr as simple as possible.I have already take a look at most of the compilation issues. Here are the corrections to solve all issues:
NAN
andINFINITE
are undeclared identifier. On VS2010, these macros are not defined. The solution would be something like specified at https://gitlab.com/embeddable-common-lisp/ecl/-/issues/282:The
malloc()
call at tinyexpr.c, line 88 should be prefixed with(te_expr *)
to castvoid*
tote_expr *
.The function
fac()
needs the declarations of the following at the beginning of the function:ncr()
needs the declarations of the following at the beginning of the function:next_token()
needs the declarations of the following at the beginning of the function:The declaration of the
ret
variable in function in functionpower()
at tinyexpr.c, line 403 needs to be moved at the beginning of the function.The declaration of the
root
variable in function in functionte_compile()
at tinyexpr.c, line 596 needs to be moved at the beginning of the function.Expression such as
te_fun2 t = s->function;
need to be type casted to compile properly. This change affects lines 431, 460, 477 and 492. The result would be the following:...
syntax in a define statement like the one at tinyexpr.c, line 82.This one is more complicated to fix and the fix is less elegant than actual code but please consider to use these corrections. I managed to fix these errors by declaring a macro for each variant of the number of argument:
and replacing usage of the
NEW_EXPR()
macro byNEW_EXPR1()
orNEW_EXPR2()
. For example, replace line 494 by the following:Since the number of calls to the
NEW_EXPR
macro is limited to 5 lines of code with 2 or 3 argument variants, I think this does not hurt the code too much.I have never contributed to a github project but if you are open to these corrections, I can modify the code and create a pull request if you wish.
Looking forward for your feedback.
The text was updated successfully, but these errors were encountered: