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

muParser identifies integers with at least 4 digits as variables #1

Closed
GoogleCodeExporter opened this issue Aug 16, 2015 · 9 comments
Closed

Comments

@GoogleCodeExporter
Copy link


What steps will reproduce the problem?
1. Set a mu::Parser's expression to one of the following:
1000*Part_0
1000.2*Part_0
0000.2*Part_0

2. Call mu::Parser::GetUsedVar()

3. 1000, 1000, and 0000 will be returned as variables for the cases listed 
above.

What is the expected output? What do you see instead?
I only expect to see Part_0 as a used variable.

-For the first case, muParser does not throw an exception, but it does say that 
1000 is a variable. I should say that it is a value.

-For the second case (1000.2*Part_0): I get the exception message: "Unexpected 
token ".2*Part_0*Part_1 " found at position 4."

-For the last case (000.2*Part_0): I get the exception message "Unexpected 
token ".2*Part_0*Part_1 " found at position 4."

muParser parses the expressions correctlywhen I use 999 and 999.2 in the 
expressions (instead of 1000 and 1000.2).

What version of the product are you using? On what operating system?
muParser 2.2.3.  I tried this with 2.2.4 taken from the svn repo and it also 
occurred. I'm on OSX version 10.7.5, and I am building muParser with the 
provided configure and make scripts. For my code, I'm compiling it with Apple 
LLVM 4.2 with C++11 support and linking to the libc++ std lib. I compiled 
muParser so that it also uses the same compiler settings as my code.

Please provide any additional information below.
I set some breakpoints in the muParser code to try and track it down. I tried 
to figure it out myself, but it's taking me a long time. Here's what I've got 
so far, and hopefully someone who knows the code better can take it from here:

In ParserBase::CreateRPN(), the line of code on line 1199:
opt = m_pTokenReader->ReadNextToken();

It returns an opt with the following state:
(mu::ParserBase::token_type) opt = {
  m_iCode = cmVAR
  m_iType = tpDBL
  m_pTok = 0x0000000100c038b8
  m_iIdx = -1
  m_strTok = "1000"
  m_strVal = ""
  m_fVal = 6.95322297490987e-310
  m_pCallback = {
    __ptr_ = 0x0000000000000000
  }
}

I believe that m_iCode should NOT be set to cmVAR, and should instead be set to 
cmVAL. I tried to dig deeper, but I didn't get very far.

Thanks for looking at this.


Original issue reported on code.google.com by kwehl...@gmail.com on 22 Aug 2013 at 11:40

@GoogleCodeExporter
Copy link
Author

I fixed the problem by editing a single line in muParserBase.cpp on line 50.

Original line:
std::locale ParserBase::s_locale = std::locale(std::locale::classic(), new 
change_dec_sep<char_type>('.'));

New line:
std::locale ParserBase::s_locale = std::locale(); 

Original comment by kwehl...@gmail.com on 10 Sep 2013 at 7:31

@GoogleCodeExporter
Copy link
Author

Does my fix break something? I'm not familiar with locales. I read up on them a 
bit, and there weren't any obvious problems with changing the line, but I don't 
know enough to say for sure that this is a good fix.

Original comment by kwehl...@gmail.com on 10 Sep 2013 at 7:32

@GoogleCodeExporter
Copy link
Author

Yes, it will break something. You won't be able to write "1,000", and get it to 
read that. They made number inputs configurable, just not very robust if you're 
not being very careful with inputs.

But it's very helpful to know you can do it (I've been trying the same thing).

I'm a total C++ beginner, but I think this would be a cleaner solution:

- Before giving the string to muParser, remove all instances of whatever the 
thousands separator ("," in English, "." in German) is from the string.

- Set the locale format to change_dec_sep(cDecSep, cThousandsSep, nGroup = 
9999). (So it won't look for thousand separators for any reasonable input). 


Original comment by peter....@gmail.com on 14 Nov 2013 at 5:37

@GoogleCodeExporter
Copy link
Author

My (quick) thought on above:
It is related to Parser::IsVal(...): the parsing of the value is stopped too 
early due to the mismatched decimal symbols '.' vs ','
Using the correct or setting the decimal separators will fix it.

Original comment by juno.kam...@gmail.com on 25 Nov 2013 at 9:47

@GoogleCodeExporter
Copy link
Author

[deleted comment]

@GoogleCodeExporter
Copy link
Author

I'll take another look at the code and see if I can get it to work. Thanks for 
the suggestions.
However, the original line of code was

std::locale ParserBase::s_locale = std::locale(std::locale::classic(), new 
change_dec_sep<char_type>('.'));

which looks like it should recognize 1000.2, because I used a "." as a decimal 
separator. I know I tried parsing 1,000.2, but I can't remember if it worked or 
not (I'm guessing it did, but I'm not sure).

Anyways, I'll just play around with that line of code, look at your 
suggestions, and read up a bit. I'll post what I learn to here.

Original comment by kwehl...@gmail.com on 25 Nov 2013 at 1:40

@GoogleCodeExporter
Copy link
Author

Sorry, I'm late for the party but i accidentaly unsubscribed from the bug 
tracker. I just tried your original sample and it seems to work properly. I'm 
using the default locale (C-locale). But even with the german local results are 
as expected.

Original comment by ib...@gmx.info on 27 Nov 2013 at 5:40

@GoogleCodeExporter
Copy link
Author

With no further followup I'm closing the issue as it is not reproduceable and 
most likely related to a client side change of the local.

Original comment by ib...@gmx.info on 22 Dec 2013 at 9:44

  • Changed state: Invalid

@GoogleCodeExporter
Copy link
Author

We are seeing this issue in an application based on deal.II 
(http://www.dealii.org) that interfaces with muParser. The issue happens on OS 
X: 3-digit numbers are ok, 4-digit numbers are not -- bizarrely, "1000" is 
rejected, "1,000" is accepted. So it is an issue of how integers are 
interpreted: it *requires* that 4-digit numbers use thousands separators, it is 
not optional.

The locales that are set are looking rather benign: they're en_US.UTF-8. Any 
clues what may be going on?

Original comment by bange...@gmail.com on 21 May 2015 at 7:33

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

No branches or pull requests

1 participant