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

Strange behavior of the postfix operator on macOS #123

Open
manu-aesim opened this issue Jul 6, 2023 · 13 comments
Open

Strange behavior of the postfix operator on macOS #123

manu-aesim opened this issue Jul 6, 2023 · 13 comments

Comments

@manu-aesim
Copy link

Hello,

I am defining postfix operators with metrics coefficients:

expression_parser.DefinePostfixOprt(_T("p"), [](double value){return value * 1E-12;});
expression_parser.DefinePostfixOprt(_T("n"), [](double value){return value * 1E-9;});
expression_parser.DefinePostfixOprt(_T("u"), [](double value){return value * 1E-6;});
expression_parser.DefinePostfixOprt(_T("m"), [](double value){return value * 1E-3;});
expression_parser.DefinePostfixOprt(_T("k"), [](double value){return value * 1E3;});
expression_parser.DefinePostfixOprt(_T("M"), [](double value){return value * 1E6;});
expression_parser.DefinePostfixOprt(_T("G"), [](double value){return value * 1E9;});

And I am using those to evaluate expressions. For some reasons that I don't understand the operators "p" and "n" are not working on macOS (Clang) but work fine on Linux (GCC) and Windows (MSVC). The other operators ("m", "u", "k"...) work on all platforms.

On macOS, the following call throw the following error message:

std::string parameter_expression = "27n";
expression_parser.SetExpr(parameter_expression);
double value = expression_parser.Eval();

error: Unexpected token "27n" found at position 0.

I don't have defined any other constant or variables and I don't have weird compiler flags. Any ideas ?

Thanks!

@manu-aesim manu-aesim changed the title Weird postfix operator behavior on macOS Strange behavior of the postfix operator on macOS Jul 6, 2023
@beltoforion
Copy link
Owner

I don't know why it is behaving that way. This would need to be debugged but i don't have a MAC and cannot do that.

@manu-aesim
Copy link
Author

Thanks for your feedback. I see the debug / dump functions. Would you like me to send any of those ?

@beltoforion
Copy link
Owner

Unfortunately I don't think that would help because I would need to step through the parsing functions to see what is wrong. I can try debugging this with LLVM on Linux. But according to my experience this will probably not show the issue. I can add a test case to the integration tests. They will be executed on a mac but i cannot debug there.

@manu-aesim
Copy link
Author

manu-aesim commented Jul 11, 2023

I understand, thank you. I don't know if it helps, but here is a Google Test that works on Windows (MSCV), Linux(GCC) but fails on macOS (Clang).

   mu::Parser expression_parser;
   expression_parser.DefinePostfixOprt(_T("n"), [](double value){return value * 1E-9;});
   std::string parameter_expression = "1n";
   expression_parser.SetExpr(parameter_expression);
   try
   {
       double value = expression_parser.Eval();
   }
   catch(mu::Parser::exception_type &e)
   {
       ASSERT_TRUE(false) << "muparser exeption:" << "\n" << "Message:  " << e.GetMsg() << "\n" \
       << "Formula:  " << e.GetExpr() << "\n" \
       << "Token:    " << e.GetToken() << "\n" \
       << "Position: " << e.GetPos() << "\n" \
       << "Errc:     " << e.GetCode() << "\n";
   }
   

@beltoforion
Copy link
Owner

beltoforion commented Aug 11, 2023

Thanks for the code. I will add it to the unit test but i still cannot debug it although the build action now shows it.

beltoforion added a commit that referenced this issue Aug 11, 2023
beltoforion added a commit that referenced this issue Aug 11, 2023
added unit test for #123
@pkatta-ktc
Copy link

I ran into a similar problem with the included tests for "1n". I tried "1 n" and the test passes. I have no idea is what I did is correct.

@pkatta-ktc
Copy link

I debugged this issue on a Mac.

In muParser.cpp, there is this function that is called during the Eval() call.

int Parser::IsVal(const char_type* a_szExpr, int* a_iPos, value_type* a_fVal)
	{
		value_type fVal(0);

		stringstream_type stream(a_szExpr);
		stream.seekg(0);        // todo:  check if this really is necessary
		stream.imbue(Parser::s_locale);
		stream >> fVal;
		stringstream_type::pos_type iEnd = stream.tellg(); // Position after reading

		if (iEnd == (stringstream_type::pos_type) - 1)
			return 0;

		*a_iPos += (int)iEnd;
		*a_fVal = fVal;
		return 1;
	}

In the above method, the line "stream >> fVal;", on a Mac does not copy the "1" from "1n" into the fVal (The value of fVal after that line is zero). That is what causes the strange behavior of the "1n" postfix operator on a Mac. On Windows, the same line causes the fVal to be populate with "1".

@beltoforion
Copy link
Owner

Thanks for debugging this. This somehow sounds familiar. I will see wether i can rewrite it without the need for a stringstream. I just need a way that works on linux, windows and mac with strings and wide strings. Maybe std::stof does the trick.

beltoforion added a commit that referenced this issue Oct 31, 2023
@pkatta-ktc
Copy link

pkatta-ktc commented Nov 1, 2023

@beltoforion I can confirm that this fixed the postfix 1n problem. I used the fix you committed in ddb6f07.

All tests pass on Mac, windows, and Linux. You can close this issue.

@manu-aesim
Copy link
Author

Thanks @beltoforion and @pkatta-ktc! I just made some tests and the post fix operators works perfectly on Mac.

@beltoforion
Copy link
Owner

beltoforion commented Dec 23, 2023

This fix is causing #136; I'll leave the fix for Macs in the code but this will break localization on Macs. I cannot easily prevent this and decided that is an APPLE problem. They need to fix the stringstream...

@al42and
Copy link
Contributor

al42and commented Feb 13, 2024

Hi! The problem also happens on Ubuntu with Clang 17 and libc++ (not with libstdc++).

With muParser a326a5e, clang 17.0.6

$ cmake .. -DCMAKE_C_COMPILER=clang-17 -DCMAKE_CXX_COMPILER=clang++-17 -DCMAKE_CXX_FLAGS=--stdlib=libc++  && make -j12
....
$ ./t_ParserTest 
testing name restriction enforcement...passed
testing syntax engine...passed
testing postfix operators...
  fail: 1n (Unexpected token "1n" found at position 0.)
  failed with 1 errors
testing infix operators...passed
testing variable/constant detection...passed
testing multiarg functions...passed
testing expression samples...passed
testing if-then-else operator...passed
testing member functions...passed
testing binary operators...passed
testing error codes...passed
testing string arguments...passed
testing bulkmode...passed
testing optimizer...passed
testing localization...passed
Test failed with 1 errors (604 expressions)

Replacing with 1 n helps.

Edit: seems related to llvm/llvm-project#18156

@beltoforion
Copy link
Owner

Thanks for posting the link to the llvm issue. I could not find any prior reports for llvm on this one.

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

No branches or pull requests

4 participants