Bug in real_parser

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug in real_parser

peterkochlarsen
There is a bug in real_parser, as an "ignored" exponent gives a wrong result. Consider

double as_double(std::string const& s)
{
    double result;
    auto begin = std::begin(s);
    if (phrase_parse(begin,std::end(s),double_,space,d)) return -1;
    return d;
}

Now as_double("045.000W") returns 45, but as_double("045.000E") returns 45000.
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

peterkochlarsen
Forgot to tell that this is for boost 1.61.0.
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

sehe
In reply to this post by peterkochlarsen
On 17-09-16 15:28, peterkochlarsen wrote:
> Now as_double("045.000W") returns 45, but as_double("045.000E") returns
> 45000.

I don't believe that for a second. Firstly the code doesn't compile (`d`
is undeclared). Secondly, on correct input it will return `-1`. I think
you need better backup to claim bugs.

Here's what I think you're trying to do, and the results are FINE if you
check the errors:

#include <boost/spirit/include/qi.hpp>
#include <iostream>

double as_double(std::string const& s)
{
    namespace qi = boost::spirit::qi;

    double d;
    auto begin = std::begin(s);
    if (qi::phrase_parse(begin, std::end(s), qi::double_/* >> qi::eoi*/,
qi::space, d))
        return d;
    return -1;
}

int main()
{
    for (std::string const& s : {
            "4",  "04",  "045.",  ".0",  "7E0",  "7.E0",
            "-4", "-04", "-045.", "-.0", "-7E0", "-7.E0",
            "+4", "+04", "+045.", "+.0", "+7E0", "+7.E0",
            "-4", "-04", "-045.", "-.0", "-7E0", "-7.E0",
            ".4e7", ".4e-7",
            // and then your case
            "045.000W",
            "045.000E",
        })
    {
        auto d = as_double(s);
        std::cout << "'" << s << "' -> " << d;
        if (d == -1)
            std::cout << " (ERR)\n";
        else
            std::cout << "\n";
    }
}

Prints

    '4' -> 4
    '04' -> 4
    '045.' -> 45
    '.0' -> 0
    '7E0' -> 7
    '7.E0' -> 7
    '-4' -> -4
    '-04' -> -4
    '-045.' -> -45
    '-.0' -> -0
    '-7E0' -> -7
    '-7.E0' -> -7
    '+4' -> 4
    '+04' -> 4
    '+045.' -> 45
    '+.0' -> 0
    '+7E0' -> 7
    '+7.E0' -> 7
    '-4' -> -4
    '-04' -> -4
    '-045.' -> -45
    '-.0' -> -0
    '-7E0' -> -7
    '-7.E0' -> -7
    '.4e7' -> 4e+06
    '.4e-7' -> 4e-08
    '045.000W' -> 45
    '045.000E' -> -1 (ERR)

If you insist that `045.000W` should fail (why?) then you should
probably write your parser to indicate it (you can also check that the
`begin` iterator is equal to `s.end()`):

    if (qi::phrase_parse(begin, std::end(s), qi::double_ >> qi::eoi,
qi::space, d))


------------------------------------------------------------------------------
_______________________________________________
Spirit-general mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/spirit-general
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

peterkochlarsen
Dear sehe,

I could not make Nabble quote your post, so you get a reply without quoting.
First, obviously the code-snippet provided was not supposed to be a self-containing compilable program, just a way to convey an idea. In creating the snippet I misnamed a variable, but I believe anyone would have understood the idea anyway.
Secondly, your code compiles perfectly on my system, but the result is different. For "045.000W" it prints 45, for "045.000E" it prints 45000. I do not understand the difference. On Coliru I get the same result.
Lastly, I expected both my examples to parse (leaving "E" or "W" for the next part of my grammar), but I did not expect "045.000" to become 45000. ;-)

/Peter
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

sehe
On 18-09-16 23:07, peterkochlarsen wrote:
On Coliru I get the same result.
I'm really unsure why you choose not to link that coliru, or say what platform/compiler it fails on.

Lastly, I expected both my examples to parse (leaving "E" or "W" for the
next part of my grammar), but I did not expect "045.000" to become 45000.

If you want to specify a parser that /does not/ expect an exponent part, then you **must** specify so. Use a custom real_parser policy.

template <typename T> struct simple_real_policies : boost::spirit::qi::real_policies<T>
{
    static bool parse_exp(...) { return false; } //  No exponent
};

See it Live On Coliru

New output:

    '4' -> 4
    '04' -> 4
    '045.' -> 45
    '.0' -> 0
    '7E0' -> -1 (ERR)
    '7.E0' -> -1 (ERR)
    '-4' -> -4
    '-04' -> -4
    '-045.' -> -45
    '-.0' -> -0
    '-7E0' -> -1 (ERR)
    '-7.E0' -> -1 (ERR)
    '+4' -> 4
    '+04' -> 4
    '+045.' -> 45
    '+.0' -> 0
    '+7E0' -> -1 (ERR)
    '+7.E0' -> -1 (ERR)
    '-4' -> -4
    '-04' -> -4
    '-045.' -> -45
    '-.0' -> -0
    '-7E0' -> -1 (ERR)
    '-7.E0' -> -1 (ERR)
    '.4e7' -> -1 (ERR)
    '.4e-7' -> -1 (ERR)
    '045.000W' -> -1 (ERR)
    '045.000E' -> -1 (ERR)


------------------------------------------------------------------------------

_______________________________________________
Spirit-general mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/spirit-general
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

peterkochlarsen
sehe wrote
If you want to specify a parser that /does not/ expect an exponent part,
then you **must** specify so. Use a custom real_parser policy.
This is not correct. The exponent should be parsed only if it is a valid exponent and  "E" is not a valid exponent as it is not followed by an integral number. At least this is my understanding based on the documentation and the comments in the code. Also, if it was indeed the case, the parser should fail and not return 45000, still exposing an error. Your example on Coliru does not check the validity of the double - it is the eoi and not the double parser, that fails.
Anyway this discussion is moot as the bug-report I filed on boost has been accepted and apparently already corrected in the develop branch.

/Peter
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

Joel de Guzman
On 19/09/2016 3:25 PM, peterkochlarsen wrote:

> sehe wrote
>> If you want to specify a parser that /does not/ expect an exponent part,
>> then you **must** specify so. Use a custom real_parser policy.
>
> This is not correct. The exponent should be parsed only if it is a valid
> exponent and  "E" is not a valid exponent as it is not followed by an
> integral number. At least this is my understanding based on the
> documentation and the comments in the code. Also, if it was indeed the case,
> the parser should fail and not return 45000, still exposing an error. Your
> example on Coliru does not check the validity of the double - it is the eoi
> and not the double parser, that fails.
> Anyway this discussion is moot as the bug-report I filed on boost has been
> accepted and apparently already corrected in the develop branch.

FWIW this is fixed now in develop.

Regards,
--
Joel de Guzman
http://www.ciere.com
http://boost-spirit.com
http://www.cycfi.com/


------------------------------------------------------------------------------
_______________________________________________
Spirit-general mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/spirit-general
Reply | Threaded
Open this post in threaded view
|

Re: Bug in real_parser

peterkochlarsen
Joel de Guzman wrote
On 19/09/2016 3:25 PM, peterkochlarsen wrote:
> sehe wrote
[SNIP]
> Anyway this discussion is moot as the bug-report I filed on boost has been
> accepted and apparently already corrected in the develop branch.

FWIW this is fixed now in develop.
I know. Can I use the space here to thank you for a wonderful library. Considering its C++98 heritage, I believe that Spirit and qi are fantastic libraries and I look forward to the day where I can use X3 with Visual Studio. Saying this, I really would love to have an update to the documentation, giving more detailed information with respect to e.g. attribute and error handling. One of the problems I am going to deal with now is how to get good error-messages.

Best regards
Peter