2

How I can add warnings in boost spirit parser.

Edit: ... that could report the issue with position

For example if I have an integer parser:

('0' >> oct)
| int_

I would like to be able to do something like this:

('0' >> oct)
| "-0" --> trigger warning("negative octal values are not supported, it will be interpreted as negative decimal value and the leading 0 will be ignored")
| int_
gsf
  • 6,612
  • 7
  • 35
  • 64
  • 1
    is a regular semantic action not an option? What are you stuck at? – sehe Oct 07 '13 at 07:01
  • It seems that semantic action is limited to the options it provides to generate reasonable feedback ... and I will need to use iterators and raw rules to collect the necessary information. on_error provides all of it for "free". I am hoping that somebody will have this solved in a more generic way. May be something based on on_error with action accept or something. – gsf Oct 07 '13 at 22:29
  • on_error only provides it "for free" because it will fail the match "for free". If you want to continue running, just use a callback of some sort or store the warning(s) in a container. – sehe Oct 07 '13 at 22:36
  • Can I create my own callback? How? – gsf Oct 07 '13 at 22:41

1 Answers1

5

Q. Can I create my own callback? How?

A. Sure. Any way you'd normally do it in C++ (or look at Boost Signal2 and/or Boost Log)

parser(std::function<bool(std::string const& s)> callback) 
    : parser::base_type(start),
      callback(callback)
{
    using namespace qi;

    start %= 
        as_string[+graph] 
            [ _pass = phx::bind(callback, _1) ]
        % +space
        ;

    BOOST_SPIRIT_DEBUG_NODES((start));
}

As you can see, you can even make the handler decide whether the warning should be ignored or cause the match to fail.


UPDATE #1 I've extended the sample to show some of the unrelated challenges you mentioned in the comments (position, duplicate checking). Hope this helps

Here's a simple demonstration: see it Live on Coliru (Word)

UPDATE #2 I've even made it (a) store the source information instead of the iterators, (b) made it "work" with floats (or any other exposed attribute type, really).

Note how uncannily similar it is, s/Word/Number/, basically: Live On Coliru (Number)

#define BOOST_RESULT_OF_USE_DECLTYPE // needed for gcc 4.7, not clang++
#define BOOST_SPIRIT_USE_PHOENIX_V3
#include <boost/spirit/include/qi.hpp>
#include <boost/spirit/include/phoenix.hpp>
#include <boost/spirit/include/phoenix_stl.hpp>
#include <functional>

namespace qi  = boost::spirit::qi;
namespace phx = boost::phoenix;

// okay, so you want position reporting (actually unrelated):
#include <boost/spirit/include/support_line_pos_iterator.hpp>
using It = boost::spirit::line_pos_iterator<std::string::const_iterator>;

// AST type that represents a Number 'token' (with source and location
// information)
struct Number 
{ 
    double      value;
    size_t      line_pos;
    std::string source;

    explicit Number(double value = 0.0, boost::iterator_range<It> const& range = {})
        : 
          value(value),
          line_pos(get_line(range.begin())),
          source(range.begin(), range.end())
    {}

    bool operator< (const Number& other) const { return (other.value - value) > 0.0001; }
};

// the exposed attribute for the parser:
using Words    = std::set<Number>;

// the callback signature for our warning; you could make it more like
// `on_error` so it takes the iterators directly, but again, I'm doing the
// simple thing for the dmeo
using Callback = std::function<bool(Number const& s)>;

template <typename It>
    struct parser : qi::grammar<It, Words()>
{
    parser(Callback warning) 
        : parser::base_type(start),
          warning(warning)
    {
        using namespace qi;
        auto check_unique = phx::end(_val) == phx::find(_val, _1);

    word   = 
               raw [ double_ [ _a = _1 ] ] [ _val = phx::construct<Number>(_a, _1) ]
               ;

        start %= 
               - word        [ _pass = check_unique || phx::bind(warning, _1) ]
               % +space
               >> eoi
               ;
    }

  private:
    Callback warning;
    qi::rule<It, Number(), qi::locals<double> > word;
    qi::rule<It, Words()> start;
};

int main(int argc, const char *argv[])
{
    // parse command line arguments
    const auto flags          = std::set<std::string> { argv+1, argv+argc };
    const bool fatal_warnings = end(flags) != flags.find("-Werror");

    // test input
    const std::string input("2.4 2.7 \n\n\n-inf \n\nNaN 88 -2.40001 \n3.14 240001e-5\n\ninf");

    // warning handler
    auto warning_handler = [&](Number const& w) { 
        std::cerr << (fatal_warnings?"Error":"Warning") 
                  << ": Near-identical entry '" << w.source << "' at L:" << w.line_pos << "\n"; 
        return !fatal_warnings;
    };

    // do the parse
    It f(begin(input)), l(end(input));
    bool ok = qi::parse(f, l, parser<It>(warning_handler));

    // report results
    if (ok)   std::cout << "parse success\n";
    else      std::cerr << "parse failed\n";
    if (f!=l) std::cerr << "trailing unparsed: '" << std::string(f,l) << "'\n";

    // exit code
    return ok? 0 : 255;
}

Prints:

Warning: Near-identical entry 'NaN' at L:6
Warning: Near-identical entry '240001e-5' at L:7
parse success
sehe
  • 374,641
  • 47
  • 450
  • 633
  • having callback from a semantic action is an option, but it does not seem sufficient for a good warning report message. When you said that I can have a callback I assumed there is a way I to create on_error type callback that could be triggered in particular situations. Such callback will be able to provide position of the warning source. – gsf Oct 09 '13 at 14:38
  • @gsf what is the difference? Look at `phx::throw_` and `phx::catch_` if you want 'decoupling' of the location where a warning is raised and where you 'handle'. However, it doesn't change a d*** thing, since a warning is a _decision_. You'll have to _decide_ to give a warning at a given time, using a given rule and using a given diagnostic message. There's no corner cutting. Except, maybe, you can make a custom parser directive to encapsulate some of the repeat machinery. Good luck. http://boost-spirit.com/home/articles/qi-example/creating-your-own-parser-component-for-spirit-qi/ – sehe Oct 09 '13 at 15:03
  • You do not see difference in a warning message that provides the location of the source of the warning and one that doesn't? – gsf Oct 09 '13 at 15:17
  • @gsf (a) why do you ask (b) what's so special about on_error? You can just pass the iterators. You already mentioned 'raw' in a comment to the original post. – sehe Oct 09 '13 at 15:18
  • As I said several times already - it gives you the whole scope of the trigger that will be sufficient to create a report message. If you think it is simple enough with semantic actions why you do not try to extend the example grammar to report a warning only if the word was already seen in the document + to provide the position. And instead of word it to be a float number, and we would like to compare it by value. 10.0 20.0 1E1 should report warning: "on line 3 the value 10.0 ("1E1") is duplicated" – gsf Oct 09 '13 at 15:33
  • 3
    @gsf I don't see how this is relevant or changes matters _essentially_. I've updated the sample (mostly with comments...) to show what you ask for. I've stopped short of comparing floats, because **a.** comparing floats for equality is a code smell and **b.** I don't have your AST with the position information, so it's not worth my time. I hope this helps you. (Note that the sample now accepts a command line option `-Werror` to treat the warning as a parse error!) – sehe Oct 09 '13 at 18:40
  • The way it matters is that now you will need the attribute and the source at the same time. Adding callback to raw gives me access to the iterators (the original source) but I will lose the attribute value. Adding callback to the actual grammar will give me access to the attribute, but not to the source. Comparing floats is an example of a very simple affix grammar that I was assuming will give you an idea while saying word "a" warning is not sufficient, because the outcome for the same word "a" could be different. – gsf Oct 10 '13 at 02:13
  • ... and at the same time was exposing the attribute/source issue. Else, talking about smell what do you think about the rule raw? Why the iterators are not just accessible in the semantic action? Is this a performance issue? – gsf Oct 10 '13 at 02:27
  • Re: "now you will need the attribute and the source at the same time" - that's always the case. Whether you store a copy of the source in the AST is _your decision_ and again, doesn't change anything essentially. You know how you already handle this, because [it's in the code for your other question](http://stackoverflow.com/questions/19215938/boost-spirit-on-error-not-triggered)... – sehe Oct 10 '13 at 07:26
  • @gsf I adapted the answer for `double`s. See **[update #2](http://coliru.stacked-crooked.com/a/cad5a9ef9bd6286b)** in the answer. I hope you now see how everything you seem to be "complaining" about is not very relevant. As you can see, it's not that hard to use `raw[]` while still storing the _exposed attribute_. What you might want to do(?) is make a custom parser directive that acts like `raw[]` but exposes ***both*** the source iterators ***and*** the attribute of the parse expression. Anyways, all this is irrelevant to the question as posed ("how to trigger a warning") – sehe Oct 10 '13 at 07:35
  • So, at this point in time, I'm going to stop guessing at your missing code. All I'm doing is saying "I don't see a problem doing X" and then repeatedly having to convince you of that by "re-inventing" your code for you. Only to find you complaining that it's not "complicated enough". Well, next time, please file a new question, and be sure to include a relevant [SSCCE](http://sscce.org/) that _demonstrates where you are stuck_. It'll waste a lot less of our time. – sehe Oct 10 '13 at 07:38
  • And, in case you didn't want to pass `Number const&` to the callback: [coliru code #3](http://coliru.stacked-crooked.com/a/ad73d7f10293c302) or or not using **any** kind of AST at all (just `std::set` with the [default comparison semantics of the builtin type](http://stackoverflow.com/questions/6684573/floating-point-keys-in-stdmap)): [coliru code #4](http://coliru.stacked-crooked.com/a/47f63d2f2742d085) – sehe Oct 10 '13 at 08:22
  • > Oh, PS. about comparing floats. Please, do not patronize me. There is no way you to not understand how irrelevant this is. – gsf Oct 10 '13 at 13:37
  • > It'll waste a lot less of our time. I do not see why you consider yourself engaged with answering my question? If you are affiliated with the boost spirit library you have to disclose it - you know that right? – gsf Oct 10 '13 at 13:44
  • My question was clear from the very beginning. I was asking for a way to report a warning that includes all the necessary information. Your answer was avoiding solving it with suggesting solutions that was overly simplified. Your code proves it. Just look at the example. Do you honestly believe that this looks like something you should do to trigger a warning message? Ok, you can, but pleeeeaaaas. – gsf Oct 10 '13 at 13:51
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/39021/discussion-between-sehe-and-gsf) – sehe Oct 10 '13 at 20:47