Skip to content
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

C11 and C++20 #71

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

C11 and C++20 #71

wants to merge 4 commits into from

Conversation

tylov
Copy link

@tylov tylov commented Nov 2, 2020

  • Replaces unsafe usages of const void* with appropriate pointer types for data and functions, making the code maximum portable, safe, and easier to maintain. Detail: Some compilers does not allow casting between data and function pointers, even void* (pointers may have different sizes).

  • Library now compilable both in c11 and c++20. Useful if you need to customize the code with a c++ allocator or use it in projects where c++ source code is required. Broadens usability of the library.

Note that it is only unnamed structs that requires C11, but unnamed structs are supported on every C99 compiler that I know of (including MSVC which isn't even C99 compliant).

  • For c++, the PR forces the correct overloaded math functions to be picked during function pointer assignments and comparisons, e.g. like pow and fmod.

  • Simplifies casting code in te_eval() by adding a few macros, can extend number of function parameters cleanly.

Replaces unsafe usages of const void* with appropriate pointer types for data and functions, making the code maximum portable, safe, and easier to maintain. Detail: Some compilers does not allow casting between data and function pointers, even void* (pointers may have different sizes).

Library now compilable both in c11 and c++20. Useful if you need to customize the code with a c++ allocator or use it in projects where c++ source code is required. Broadens usability of the library.

For c++, the PR forces the correct overloaded math functions to be picked during function pointer assignments and comparisons, e.g. like pow and fmod.

Simplifies casting code in te_eval() by adding a few macros, can extend number of function parameters cleanly.
… cbrt(), log2(), gcd() math functions. Removed ln(): now log(), log2() and log10().
@bavay
Copy link
Contributor

bavay commented Oct 4, 2021

Great, I'm looking forward to a C++11 / C++20 compatible tinyexpr! This could make the compilation environment a little easier when integrating tinexpr in a C++11 project (ie no need to check on multiple platforms and compilers that this one file will be compiled as c99 vs the other ones as c++11).

Copy link

@SirNate0 SirNate0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. I care less about the ** instead of ^, but I think the changes to use separate function pointer types should be completed to allow up to 7 arguments if that route is preferred. Though at that point I think the closure types should also be broken out, as that would have a separate type with the void* for the context.

@@ -53,9 +56,9 @@ enum {

typedef struct te_variable {
const char *name;
const void *address;
union {const double *address; te_fun0 fun0; te_fun1 fun1; te_fun2 fun2;};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are splitting the address out into different specific function types, why not include all the way up to 7 arguments? And if not, why not just leave it as is with a single void* function and cast for all of them? Currently, I don't think the tinyexpr code uses any functions with more than 2 arguments, but a simple and relatively common example would be a linear interpolation (lerp) function, or an if function like excel's if(condition,then,else).

@@ -101,7 +102,7 @@ After you're finished, make sure to call `te_free()`.

int err;
/* Compile the expression with variables. */
te_expr *expr = te_compile("sqrt(x^2+y^2)", vars, 2, &err);
te_expr *expr = te_compile("sqrt(x**2+y**2)", vars, 2, &err);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change to ** from ^? I don't necessarily oppose it, using Python frequently myself, but it's certainly not a mathematical standard practice (Mathematica, Matlab).

@@ -244,7 +245,7 @@ TinyExpr parses the following grammar:
<list> = <expr> {"," <expr>}
<expr> = <term> {("+" | "-") <term>}
<term> = <factor> {("*" | "/" | "%") <factor>}
<factor> = <power> {"^" <power>}
<factor> = <power> {"*\*" <power>}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than having "*\*" in the markdown, it might make more since to wrap the section in a code block, if that was the reason to use *\* instead of **.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants