Here's a second answer to address (some of the) the XY-problem(s) you are probably trying to solve.
As I noted in a comment there are a number of code smells[1] in your example. Let me explain what I mean.
The Goal
Let's consider what the goal of the program is: You're doing input parsing.
The result of parsing should be data, preferrably in C++ datatypes with strong type information, so you can avoid working with quirky (possibly invalid) variable text representations and focus on business logic.
The Smells
Now on to the smells:
You define "abstract datatypes" (like NumValue) but then you fail to use them consistently:
typedef double NumValue;
typedef boost::variant<double, std::wstring> GenericValue;
// ^--- should be NumValue
Be more consistent, and make your code reflect the design:
namespace ast {
typedef double Number;
typedef std::wstring String;
typedef boost::variant<Number, String> Value;
}
You use a parser generator for the parsing, yet you are also invoking
boost::lexical_cast<double> on ... stringswostringstream, from which you (forgetting std::ios::skipws...) extract "a" stringboost::iequals to compare strings, that should already have been parsed into their strongly-typed AST types, independent of letter-case.You have a static_visitor to act on variant types, yet you rely on stringification (using wostringstream). In fact, you only ever call the visitor on that variant iff you already know that it's a number:
else if(val.which() != 1 && CheckIfNumValueIsNull(Num(val)))
That's a bit funny, because in this case you could just have used boost::get<NumValue>(val) to get the known-type value out.
Pro Tip: Using "lowlevel" parsing/streaming operations while using a high-level parser-generator, is a code smell
Your generic value variant suggests that your grammar supports two kind of values. Yet, your grammar definition clearly shows that you have a third type of value: %null%.
There's evidence that you got somewhat confused by this yourself, as we can see the parser
- parsing the literal
%null% (or %NULL% etc) into ... some kind of magic number.
- therefore, we know that iff
%null% was parsed, it will always be a NumValue in your AST
- we also know that strings will always be parsed into a
wstring subtype GenericValue
- yet, we can see you treat any
GenericValue as potentially null?
All in all it leads to a rather surprising...
Summary: You have AsNumValue which you (ironically) appear to be using to find out whether a String might actually be Null
Hint: a String could never represent the %null% to begin with, it makes no sense to transform random strings into numbers, and random 'magic numeric values' should not have been used to represent Null in the first place.
Your grammar makes unbalanced use of semantic actions:
factor_ =
(no_case[ParserNullChar]) [_val = NumValueDoubleNull]
| double_ [ _val = _1]
| string_ [ _val = _1]
| function_call_ [ _val = _1]
;
We notice that you are simultaneously
- using SA's to manually perform what automatic attribute propagation is supposed to do (
[_val = _1])
- using a single branch for "magic" purposes (this is where you require a
Null AST datatype)
In my suggested solution below, the rule becomes:
factor_ = null_ | string_ | double_ | function_call_;
That's it.
Pro Tip: Using Semantic Actions sparingly (see also Boost Spirit: "Semantic actions are evil"?)
The Solution
All in all, plenty of room to simplify and cleanup. In the AST department,
- Extend the Value variant with an explicit
Null subtype
- Rename types and move into a namespace for readability
- Drop the
AsNumValue function which has no purpose. Instead, have an IsNull visitor that just reports true for Null values.
namespace ast {
typedef double Number;
typedef std::wstring String;
struct Null {};
typedef boost::variant<Null, Number, String> Value;
struct IsNull
{
typedef bool result_type;
template <typename... T>
constexpr result_type operator()(T const&...) const { return false; }
constexpr result_type operator()(Null const&) const { return true; }
};
}
In the Grammar department,
Factor the grammar into rules that match the AST nodes
qi::rule<It, ast::String()> string_;
qi::rule<It, ast::Number()> number_;
qi::rule<It, ast::Null()> null_;
qi::rule<It, ast::Value(), Skipper> factor_, expr_, function_call_;
This makes your grammar simple to maintain and to reason about:
string_ = (L'"' > *~char_('"') > L'"')
| (L"'" > *~char_("'") > L"'")
;
number_ = double_;
null_ = no_case["%null%"] > attr(ast::Null());
factor_ = null_ | string_ | double_ | function_call_;
expr_ = factor_;
BOOST_SPIRIT_DEBUG_NODES((expr_)(factor_)(null_)(string_)(number_)(function_call_))
Note it makes debug output more informative too
I've taken the liberty to rename TakeOne to Coalesce[2]:
function_call_ = no_case[L"Coalesce"]
> ('(' > expr_ % ',' > ')') [ _val = Coalesce_(_1) ];
This is still using the approach like I showed in the other answer, only, the implementation has become much simpler because there is no longer so much confusion about what could be Null
Take Away: Values that are Null are just... Null!
Removing the now unused header includes, and adding a load of test inputs:
int main()
{
typedef std::wstring::const_iterator It;
MapFunctionParser<It, boost::spirit::qi::space_type> parser;
for (std::wstring input : {
L"Coalesce()",
L"Coalesce('simple')",
L"CoALesce(99)",
L"CoalESCe(%null%, 'default')",
L"coalesce(%null%, -inf)",
L"COALESCE(%null%, 3e-1)",
L"Coalesce(%null%, \"3e-1\")",
L"COALESCE(%null%, 5, 'default')",
L"COALESCE(%NULL%, %null%, %nuLl%, %NULL%, %null%, %null%, %nUll%, %null%, %NULL%, %nUll%, %null%, \n"
L"%NULL%, %NULL%, %null%, %null%, %nUll%, %null%, %nUll%, %nULl%, %null%, %null%, %null%, \n"
L"%null%, %nUll%, %NULL%, %null%, %null%, %null%, %null%, %null%, %null%, %nUll%, %nulL%, \n"
L"%null%, %null%, %nUll%, %NULL%, 'this is the first nonnull', %nUll%, %null%, %Null%, \n"
L"%NULL%, %null%, %null%, %null%, %NULL%, %null%, %null%, %null%, %NULL%, %NULL%, %nuLl%, \n"
L"%null%, %null%, %nUll%, %nuLl%, 'this is the default')",
})
{
It begin(input.begin()), end(input.end());
ast::Value result;
bool ret = phrase_parse(begin, end, parser, qi::space, result);
std::wcout << std::boolalpha << ret << ":\t" << result << std::endl;
}
}
We can now test the parsing, evaluation and error-handling:
Error! Expecting <list><expr_>"," here: ")"
false: %null%
true: simple
true: 99
true: default
true: -inf
true: 0.3
true: 3e-1
true: 5
true: this is the first nonnull
See it Live On Coliru
Without the extra test cases it takes about 77 lines of code, less than half of your original code.
Full Code
For future reference
//#define BOOST_SPIRIT_DEBUG
#define BOOST_SPIRIT_USE_PHOENIX_V3
#include <boost/spirit/include/qi.hpp>
#include <boost/spirit/include/phoenix.hpp>
#include <boost/phoenix/function/adapt_function.hpp>
namespace qi = boost::spirit::qi;
namespace phx = boost::phoenix;
namespace ast {
typedef double Number;
typedef std::wstring String;
struct Null {
friend std::wostream& operator<<(std::wostream& os, Null) { return os << L"%null%"; }
friend std:: ostream& operator<<(std:: ostream& os, Null) { return os << "%null%"; }
};
typedef boost::variant<Null, Number, String> Value;
struct IsNull
{
typedef bool result_type;
template <typename... T>
constexpr result_type operator()(T const&...) const { return false; }
constexpr result_type operator()(Null const&) const { return true; }
};
Value Coalesce(std::vector<Value> const& arglist) {
for (auto& v : arglist)
if (!boost::apply_visitor(IsNull(), v))
return v;
//
if (arglist.empty())
return Value(Null());
else
return arglist.back(); // last is the default, even if Null
}
}
BOOST_PHOENIX_ADAPT_FUNCTION(ast::Value, Coalesce_, ast::Coalesce, 1)
template <typename It, typename Skipper = qi::space_type >
struct MapFunctionParser : qi::grammar<It, ast::Value(), Skipper>
{
MapFunctionParser() : MapFunctionParser::base_type(expr_)
{
using namespace qi;
function_call_ = no_case[L"Coalesce"]
> ('(' > expr_ % ',' > ')') [ _val = Coalesce_(_1) ];
string_ =
(L'"' > *~char_('"') > L'"')
| (L"'" > *~char_("'") > L"'");
number_ = double_;
null_ = no_case["%null%"] > attr(ast::Null());
factor_ = null_ | string_ | double_ | function_call_;
expr_ = factor_;
on_error<fail> (expr_, std::cout
<< phx::val("Error! Expecting ") << _4 << phx::val(" here: \"")
<< phx::construct<std::string>(_3, _2) << phx::val("\"\n"));
BOOST_SPIRIT_DEBUG_NODES((expr_)(factor_)(null_)(string_)(number_)(function_call_))
}
private:
qi::rule<It, ast::String()> string_;
qi::rule<It, ast::Number()> number_;
qi::rule<It, ast::Null()> null_;
qi::rule<It, ast::Value(), Skipper> factor_, expr_, function_call_;
};
int main()
{
typedef std::wstring::const_iterator It;
MapFunctionParser<It, boost::spirit::qi::space_type> parser;
for (std::wstring input : {
L"Coalesce()",
L"Coalesce('simple')",
L"CoALesce(99)",
L"CoalESCe(%null%, 'default')",
L"coalesce(%null%, -inf)",
L"COALESCE(%null%, 3e-1)",
L"Coalesce(%null%, \"3e-1\")",
L"COALESCE(%null%, 5, 'default')",
L"COALESCE(%NULL%, %null%, %nuLl%, %NULL%, %null%, %null%, %nUll%, %null%, %NULL%, %nUll%, %null%, \n"
L"%NULL%, %NULL%, %null%, %null%, %nUll%, %null%, %nUll%, %nULl%, %null%, %null%, %null%, \n"
L"%null%, %nUll%, %NULL%, %null%, %null%, %null%, %null%, %null%, %null%, %nUll%, %nulL%, \n"
L"%null%, %null%, %nUll%, %NULL%, 'this is the first nonnull', %nUll%, %null%, %Null%, \n"
L"%NULL%, %null%, %null%, %null%, %NULL%, %null%, %null%, %null%, %NULL%, %NULL%, %nuLl%, \n"
L"%null%, %null%, %nUll%, %nuLl%, 'this is the default')",
})
{
It begin(input.begin()), end(input.end());
ast::Value result;
bool ret = phrase_parse(begin, end, parser, qi::space, result);
std::wcout << std::boolalpha << ret << ":\t" << result << std::endl;
}
}
[1] Origin? http://c2.com/cgi/wiki?CodeSmell (maybe it was Kent Beck?)
[2] Coalesce referring to corresponding functions in some programming languages
