Closed Xeverous closed 5 years ago
X3 requires existing code to change grammar to add more
const x3::unused_type
workarounds
I do not see where you added more workarounds.
The problem seems that you have to remove const
after the change and that was intended (actually you need to completely remove the third parameter). I did not add unused_type const
specialization because of: 1) maintaining backward comparability with such workaround will make code more complex without much need 2) that workaround is fragile as it makes has_attribute == true
.
The users that upgrade to a new version can just remove it.
Try to add this:
boost::spirit::x3::detail::rule_attr_deducer<unused_type const>
: rule_attr_deducer<unused_type> {};
If it will work for you I will add it not to bother people with changing their code.
I tried the const
workaround and it looks like it still works. Your code seems to uncover some other bug, will look into it.
Well, that's exactly what I meant to ban: if a parser with has_attribute==true assigns to a rule - that's look suspicious and there should be missing omit
, but now I see that requiring omit
is not practical.
Hmm, something strange happens here, I will need more time to dig into this. BOOST_SPIRIT_INSTANTIATE(grammar_type, iterator_type, context_type)
somehow leads to instantiation of parse_rule
for whitespace_type
with std::string
attribute, I do not understand how it happens.
On 2/6/19 6:47 PM, Nikita Kniazev wrote:
Hmm, something strange happens here, I will need more time to dig into this.
BOOST_SPIRIT_INSTANTIATE(grammar_type, iterator_type, context_type)
somehow leads to instantiation ofparse_rule
withstd::string
attribute, I do not understand how it happens.
I think because grammar_type::attribute_type is std::string and, according to what I see on my checkout of that commit, that's exactly what BOOST_SPIRIT_INSTANTIATE does:
template bool parse_rule
< Iterator, Context,
rule_type::attribute_type //std::string
>
(...
So where's the mystery?
I edited my message because I missed "for whitespace_type
". Run the code or open GitHub to see it.
I think this happens because of ADL/overload resolution. I was thinking previously why parse_rule
is not hidden under rule itself, looks like it was a useful idea.
I tried the
const
workaround and it looks like it still works. Your code seems to uncover some other bug, will look into it.
Yes, the workaround still works but now I have to place const x3::unused_type
in more places. Code without any unused type in templates does not compile.
Removing all const x3::unused_type
and x3::unused_type
does not solve the issue - try with the code I posted.
Thanks a ton for catching this!
Looks like I will be the bug hunter here. And all of this started accidentally when making a hobby parser project to automate some config generation tasks.
The project isn't even at 30% completion and yet, every few commits I encounter weird compilation error or a runtime bug. Most of X3 bugs are catched by the compiler.
I think you can expect more issues from me in the future, I haven't even started writing unit tests for my program.
Thanks for the fast fix of this, I will test whether I can drop all of current X3 workarounds in my project.
Looks like I will be the bug hunter here. ... I think you can expect more issues from me in the future
That's fine. It is cool that you are using develop branch, I am trying not to break it, but the test suite cannot be perfect so I greatly appreciate any additional testing.
I am using develop branch because the code from official Boost release does not compile. You can view my project as an additional library testing. I don't mind breaking changes. Nonetheless I'm surprised about some bugs ... I would not expect that noone has earlier tried non-copyable types in AST. Apparently there are very few users of X3 yet.
There were, but no one dig into what happen and it was not on my priority list. I spend more than 10 hours to sort out all the underlying problems and to make a good fix.
It is just not a typical use case, with either forward_ast or unique/shared pointers it would not be a problem. I also received a green light from boost::variant maintainer for making recursive_wrapper more like forward_ast (but currently I spend time here =).
after a3cf3f2c3 X3 requires existing code to change grammar to add more
const x3::unused_type
workaroundsThis compiles fine just before a3cf3f2c3:
but after that commit, all grammars that do not syntesize any attribute have to specify
const x3::unused_type
(uncommentconst
)Unfortunately the pull mentioned in https://stackoverflow.com/a/54095167/4818802 does not solve the problem, it only requires code to have more workarounds than previously.