boostorg / spirit

Boost.org spirit module
http://boost.org/libs/spirit
393 stars 161 forks source link

Why not rm the SIOF by using template metaclass specialized on rule_id? #643

Closed cppljevans closed 3 years ago

cppljevans commented 3 years ago

Seth proposed a solution to a problem appearing on stackoverflow here:

https://github.com/sehe/splitting-large-x3-parsers

Another solution was proposed here:

https://github.com/sehe/splitting-large-x3-parsers/issues/1

which eliminated the SIOF problem with Seth's solution. Since Seth hasn't responded to that proposed solution, I was hoping someone else might; hence, I'm announcing it here. Note, this proposed solution would eliminate the need for the rule_definition; hence, would break any existing spirit-x3 code that uses it, but that might be avoided using some sort of #pragma.

Kojoley commented 3 years ago

Do you have an MWE? I barely guess what is the issue. SO question has no complete example, and the code there for unknown reasons defines rule placeholder and rule definition as extern variables:

x3::rule<class rule_tag, attribute, true> rule_name = "rule_name";

auto rule_name_def = some_definition;

There are no reasons to do that.

cppljevans commented 3 years ago

On 2/3/2021 1:49 PM, Nikita Kniazev wrote:

Do you have an MWE? I barely guess what is the issue. SO question has no complete example, and the code there for unknown reasons defines rule placeholder and rule definition as extern variables:

x3::rule<class rule_tag, attribute,true> rule_name = "rule_name";

auto rule_name_def = some_definition;

There are no reasons to do that.

Nikita,

Thanks for responding.

Although the SO thread:

https://stackoverflow.com/questions/61800010/is-this-the-correct-way-of-defining-a-set-of-recursive-rules

makes no mention of the SIOF problem, the <continue this discussion in chat> does.  I should have mentioned this, but I had assumed you would have thoroughly read the SO thread and noticed the chat link and read the chat contents and consequently noticed the SIOF problem observed by Seth here:

  https://chat.stackoverflow.com/transcript/message/49395746

where he says:

  Sadly, we cannot use BOOST_SPIRIT_DEFINE anymore, BUT this is   something we might submit to upstream as the initialization fiasco   is a long-standing issue AFAIK

Now, Seth does use the acronym, "AFAIK"; so, maybe Seth is wrong about there being a SIOF issue, but I'd guess, based on Seth's extensive and quality replies on SO and here, that he pretty much knows what he's talking about ;)

Now, as far as a MWE demonstrating the SIOF problem, I don't have one; however, Seth, since he actually observed this SIOF problem, maybe can provide one.  I'd suggest getting in touch with him to get a MWE demonstrating the SIOF problem.  OTOH, Seth does provide support for the "long-standing" qualifier when he links, in his comments to:

https://stackoverflow.com/questions/59865577/how-to-deal-with-boost-spirit-x3-causing-a-static-initialization-order-fiasco/59865878#59865878

Hence, I assumed you were familiar with the problem and could reproduce the "long-standing" problem fairly easily.

If you further follow the chat, you'll see I (user1681377) guessed maybe the static storage qualifier within the macro generated parse_rule was causing the SIOF problem:

  https://chat.stackoverflow.com/transcript/message/49518618

Nikita, is static storage qualifier at least a "contributing" reason for the SIOF problem which Seth says is a "long-standing issue"?  If so, then the proposed "defn_gram" solution:

  https://github.com/sehe/splitting-large-x3-parsers/issues/1

might solve the problem since it requires no static storage. Instead, the macro generated parse_rule specialization body would just retrieve the RHS type from the defn_gram::type generated by the DEFN_GRAM specialization shown in the aforementioned splitting-large-x3-parsers/issues/1, then the attribute type could be generated as usual from that RHS grammar type, and then parse_rule would proceed just as before.  NOTE, I haven't actually tried this, I'm only, again, guessing.  However, I have actually done something similar with a much different design, as you might infer if you actually looked at the DEFN_GRAM macro and the defn_gram forward declaration from:

  https://github.com/sehe/splitting-large-x3-parsers/issues/1

In addition, since you, Nikita, are much more expert in spirit-x3, you'd be much more able to produce an acceptable test case showing whether or not the "defn_gram" proposal works; hence, I'll leave it to you to do that since, AFAICT, I've provided sufficient details of the "defn_gram" solution that you can easily "fill in the blanks" ;)

-regards, Larry

Kojoley commented 3 years ago

I did read all the links, and the mentioned chatroom too. I was even writing a paragraph about guessing what you might thinking of as an issue and that I do not like that line in the rule definition macro either, but there is nothing wrong in it, and I believe it is a leftover after a rule definition redesign (https://github.com/boostorg/spirit/blob/4afe8de7cbe7b4d21b8d0049c12ca1a2ed0c9a08/example/x3/calc9/common.hpp vs https://github.com/boostorg/spirit/blob/develop/example/x3/calc/calc9/common.hpp). I do not know of any "SIOF problem" in the library, but I do know that people use the library in such a way that they introduce a SIOF in their own code, what I can classify as a documentation/example issue, and which I address in one my pull requests but ended in shutting my efforts down after encountering an wording/naming argue.