{"action":"created","issue":{"url":"https://api.github.com/repos/boostorg/spirit/issues/200","repository_url":"https://api.github.com/repos/boostorg/spirit","labels_url":"https://api.github.com/repos/boostorg/spirit/issues/200/labels{/name}","comments_url":"https://api.github.com/repos/boostorg/spirit/issues/200/comments","events_url":"https://api.github.com/repos/boostorg/spirit/issues/200/events","html_url":"https://github.com/boostorg/spirit/pull/200","id":167979240,"number":200,"title":"Support for n-ary named operators","user":{"login":"mxc-commons","id":5009831,"avatar_url":"https://avatars.githubusercontent.com/u/5009831?v=3","gravatar_id":"","url":"https://api.github.com/users/mxc-commons","html_url":"https://github.com/mxc-commons","followers_url":"https://api.github.com/users/mxc-commons/followers","following_url":"https://api.github.com/users/mxc-commons/following{/other_user}","gists_url":"https://api.github.com/users/mxc-commons/gists{/gist_id}","starred_url":"https://api.github.com/users/mxc-commons/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mxc-commons/subscriptions","organizations_url":"https://api.github.com/users/mxc-commons/orgs","repos_url":"https://api.github.com/users/mxc-commons/repos","events_url":"https://api.github.com/users/mxc-commons/events{/privacy}","received_events_url":"https://api.github.com/users/mxc-commons/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"djowel","id":568094,"avatar_url":"https://avatars.githubusercontent.com/u/568094?v=3","gravatar_id":"","url":"https://api.github.com/users/djowel","html_url":"https://github.com/djowel","followers_url":"https://api.github.com/users/djowel/followers","following_url":"https://api.github.com/users/djowel/following{/other_user}","gists_url":"https://api.github.com/users/djowel/gists{/gist_id}","starred_url":"https://api.github.com/users/djowel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/djowel/subscriptions","organizations_url":"https://api.github.com/users/djowel/orgs","repos_url":"https://api.github.com/users/djowel/repos","events_url":"https://api.github.com/users/djowel/events{/privacy}","received_events_url":"https://api.github.com/users/djowel/received_events","type":"User","site_admin":false},"assignees":[{"login":"djowel","id":568094,"avatar_url":"https://avatars.githubusercontent.com/u/568094?v=3","gravatar_id":"","url":"https://api.github.com/users/djowel","html_url":"https://github.com/djowel","followers_url":"https://api.github.com/users/djowel/followers","following_url":"https://api.github.com/users/djowel/following{/other_user}","gists_url":"https://api.github.com/users/djowel/gists{/gist_id}","starred_url":"https://api.github.com/users/djowel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/djowel/subscriptions","organizations_url":"https://api.github.com/users/djowel/orgs","repos_url":"https://api.github.com/users/djowel/repos","events_url":"https://api.github.com/users/djowel/events{/privacy}","received_events_url":"https://api.github.com/users/djowel/received_events","type":"User","site_admin":false}],"milestone":null,"comments":11,"created_at":"2016-07-27T23:21:05Z","updated_at":"2016-08-09T02:21:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/boostorg/spirit/pulls/200","html_url":"https://github.com/boostorg/spirit/pull/200","diff_url":"https://github.com/boostorg/spirit/pull/200.diff","patch_url":"https://github.com/boostorg/spirit/pull/200.patch"},"body":"This PR adds support for n-ary named operators. They contribute to the nary_parser concept.\r\n\r\nThe implementation extends the qi metagrammar to accept `proto::function` tags. If a `proto::function` tag gets encountered together with an appropriate specialization of `use_function` a  generator function named `make_function` gets called which produces a nary parser component.\r\n\r\nThe parser class enabled here uses function style syntax. The general form is `op(p1, ...)`, where `op` is the name of the nary parser and `(p1, ...)` are a variadic number of arguments. Each argument must conform to the qi metagrammar, i.e. it has to be a qi parser expression.\r\n\r\nThis PR is intended to add the capability to define n-ary named operators. It does not introduce a particular operator yet.\r\n\r\nA test case is provided with test/function.cpp. This file in particular implements a ternary conditional operator `if_`. Purpose of this implementation is to test if a nary operator gets created properly, i.e. that the additions to the qi core work properly. The other purpose is to demonstrate how nary named parsers get implemented and enabled via `use_function`.\r\n\r\nThe test case compiles and runs without failure on msvc-9.0, msvc-12.0, msvc-14.0, gcc 6.1. The complete test suite does not compile currently as discussed already in PR #198. With PR #201 I provided a bug fix for the qi part of the test suite. The introduction of nary operators has no impact on the tests in their current state. What compiled and ran before nary operators where introduced continues to compile and run. No additional fails. With the PR #201 bugfix, the qi test suite compiles with no errors or failures.\r\n\r\nThe only adjustment to the documentation is the What's New section which announces support for\r\nthis parser class. This might be ok at this stage (without having a particular operator introduced).\r\n\r\nHere is a list of the particular changes:\r\n\r\n - Added to What's New documentation\r\n - Added an additional case for `proto::function` to the metagrammar.\r\n - Added `make_function` handler for this particular case.\r\n - Renamed function `make_binary_helper` to `make_nary_helper`. This helper function is suitable for both binary and nary operators.\r\n - Created test case function.cpp.\r\n - Added test case to jamfile.\r\n\r\n\r\nMay I ask the community to please assess this pull request, the discussion, and if you are deep in qi the code provided, also? I would highly appreciate if you respond to this post and tell me, what you think.\r\n\r\nHere is what I think why this PR should be accepted:\r\n\r\n1. Currently the only option you have to write a qi operator, is to implement a parser and enable it using use_operator<>.\r\n\r\n     The arity is restricted to unary and binary and you have to associate your parser with a valid C++ operator like (examples: -y, x % y).  You can not write a parser operator with an arity > 2 with qi. \r\n\r\n    **With this PR you can write write operators of arbitrary (even variadic) arity. Fully C++03 compliant.**\r\n\r\n2. Qi already supports the nary_parser model in a way which is not per se restricted to unary and binary operators. That means if there was way to make an n-ary operator to be recognized by qi, qi would fully support it. \r\n\r\n    **This PR provides a way to make qi know about n-ary operator parsers. Not more. The existing code base is not touched.**\r\n\r\n3. The number of c++ operators available is limited. So currently the maximum number of unary parsers which can get implemented is equal to the number of unary c++ operators. The same applies for binary operators and binary c++ operators.\r\n\r\n    **With this PR the number of possible unary and binary parsers is not limited any longer.**\r\n\r\n4. If you ever implemented a qi operator parser, you may have found, that it is not quite easy to select a c++ operator suitable to look expressive and meaningful in terms of what your parser does. \r\n\r\n    **With this PR you can provide a meaningful name (even) for your unary or binary parser.**\r\n\r\n5. Another topic is operator precedence. If you write `a | b >> c % d`, then c++ operator\r\nprecedence applies, because this is valid c++ code getting executed. If you want\r\nto implement a binary parser this makes the selection of an operator even more\r\ndifficult (see http://en.cppreference.com/w/cpp/language/operator_precedence).\r\nMost of more \"interesting\" operators (those with higher prio (lower number)) are already used by qi.\r\n \r\n    **With this PR you can write operators which have a precedence of 2 (function call).**\r\n\r\n6. Making my first dives into qi I was looking in particular for differences between qi and X3. I also asked Joel, if qi is going to get deprecated. He said it is not. Therefore it makes sense to examine X3 for interesting things qi is lacking. I found the  expect directive was a good case to learn. I created one for qi (PR#196) which got accepted. Support for n-ary operators is more tricky, but it is also a feature\r\nX3 supports.\r\n\r\n    See this example http://coliru.stacked-crooked.com/a/cb13c1d357cdc464. This was gently provided      by sehe as a part of one of my silly questions on stackoverflow.\r\n\r\n    **This PR closes a capability gap between qi and X3. As qi is not going to get deprecated, this seems  to be  important.**\r\n\r\n7. Rule evaluation control: qi automatically applies rules and that works good. Anyway, there seems to be (have been) a demand by users to gain some control about the rule evaluation control flow. Best example for that is the legendary Nabialek trick which is still referenced today.\r\n\r\n    This PR allows you to create a parser which you pass a variadic number of parsers in. Then your parser is in full control of what it does with the supplied argument parsers. You could even implement your own rule application algorithm. \r\n\r\n    In extreme: Create a parser, say my_engine, pass all your rules in and evaluate them to your liking completely bypassing the qi rule application algorithm. Without loosing any of qi's features. Maybe no one would actually want to drive that into the extreme. But there are always many approaches possible to solve a problem. \r\n\r\n   C++ offers so many different concepts to approach a problem. Control is up to the user. The user can decide and is not restricted to a particular programming paradigm or implementation by the programming environment.\r\n\r\n    **This PR increases the options you have to approach a problem using qi significantly.**\r\n\r\n8. Is there an alternative solution?\r\n\r\n    Qi binary and unary operators cover partly what this PR introduces. They cover the operator part of the functionality. But you can't name them, their number is limited to number of c++ operators available and their precedence depends on the c++ operator precedence they are associated with. Existing Qi operators are great, in particular operator notation is often quite beautiful. Qi operators have limitations which this PR addresses.\r\n\r\n    **This PR removes limitations restricting the implementation of unary and binary parser operators. Operators of arbitrary arity can not get emulated using existing qi features.**\r\n\r\n9. Does qi get more expressive? \r\n\r\n    Qi gets more expressive in terms of HOW things can get parsed. It gets more expressive in that operators of arbitrary (even variadic) arity can be defined. Which currently can not be done. There is no way to emulate an operator with variadic arguments with qi. The only option is cascading binary operators. `p1 >> p2 >> p3 >> ...`.  Depending on the particular implementation a native operator with variadic argument list can perform better.\r\n\r\n    Even with operator chaining you do not have a central point of control over the argument parsers. No `>>` does know of it's neighbour `>>`'s  second argument. They are not context aware.\r\n\r\n    **This PR allows to create context aware operators of arbitrary arity.**\r\n\r\n10. Windows-MSVC: I am on that platform and in most project contexts I do not have the choice to switch the compiler. Enterprises with PSPGs determining the full tool chain. So for me, and maybe to some of you, MSVC support is important. X3 is not necessarily aimed at full MSVC support (i.e. see discussion here: PR #168 ). I want to please ALL of my customers with THE SAME version of spirit. I can not afford and I do not see any justification to enable my people to support X3 for customers, who don't care about MSVC and qi for customers who do. If X3 does not support MSVC, it's a no go. So a modern qi is important.\r\n\r\n    **This PR enhances the capabilities of qi by bringing them closer to X3, which is important for people on MSVC.**\r\n\r\n11. What would be the risks imposed by merging this PR?\r\n\r\n    This PR touches the metagrammar, the heart of qi. This might frighten on the first sight. But much more than location does structure contribute to the possible risk imposed. Let's see:\r\n\r\n    The metagrammar extension with an additional switch for proto::tag functions does never fire that switch with existing qi code. Proof: If any of the existing qi constructs would have produced a `proto::tag function` (this would fire the switch), this code would not have compiled. You can try this out. So we can assume that none of the parser implementations out in the community produces a `proto::tag::function`. Therefore the new switch will not fire with any existing code. qed. This means in particular, existing code out there will produce exactly the same parsers as before. No reinterpretation possible.\r\n\r\n   If the switch never fires, the `make_function` generator function will never get triggered by existing code. Existing qi code can never instatiate a `function` parser by failure.\r\n\r\n    The renaming of `make_binary_helper` to `make_nary_helper` does not induce risk. The only reference to that was also changed. I changed the name, because this helper supports nary parser creation in general, not only in the particular binary case. I think this rename makes sense anyway. The new name is more precise. \r\n\r\n    So IMHO there is no risk for existing code. This assumes, that C++ is deterministic. \r\n\r\n    The only chance I see that the pure existance of this PR's code would harm existing code, is that it could reveal bugs which already existed in qi but did not show up (the compiler does something different, which may lead to a different memory layout, which could change something which was undefined but harmless to something undefined and harmful). Allthough that minimal risk exists, I believe it is neglectible. Reason: Qi compiles cross-platform cross-compiler. Such a sophisticated bug would have shown up on some platform on some compiler. \r\n\r\n  So the changes are fundamental. A new class of parsers is a fundamental enhancement. The change had to be done deep in the core. The metagrammar was extended by an additional switch.  But nothing else actually gets changed. Something is added which does not interfere with what was there already. The main reason that this solution could be done which seamlessly integrates in existing meta-grammar/meta-compiler code is, that qi is so well done, that support for n-ary operators was already available. Just like as if someone actually anticipated upcoming n-ary operators.\r\n\r\n   It remains the risk of using the new feature. But this only means any new `function` has to pass QA by passing tests as required. The same procedure as for all parsers.\r\n\r\n   **My conclusion:  This PR does not induce any significant risk.**\r\n\r\n12. Another risk: If qi stops to evolve, community support will erode.\r\n    **That's for sure.**\r\n\r\n13. Do we need this enhancement? \r\n    It depends on what you mean by \"need\". We don't need computers but life is better with them. Earth will not stop spinning without this PR. I'd love to see, that qi is not end of live, so: \r\n\r\n    **If it is true that qi is not going to get deprecated then what is important for X3 should be important for qi also. And considering the points above my answer is: Yes.** \r\n\r\n   \r\n\r\nSo, what do you think? Thank you for the time you spent on reading this. Any feedback would be highly appreciated. Did I miss something important? If you want to test this, checkout the develop branch of my fork of spirit. develop is actually the feature branch for this PR (I learned somewhat late about branching ;) (https://github.com/mxc-commons/spirit)\r\n\r\nRegards, Frank\r\n"},"comment":{"url":"https://api.github.com/repos/boostorg/spirit/issues/comments/238435123","html_url":"https://github.com/boostorg/spirit/pull/200#issuecomment-238435123","issue_url":"https://api.github.com/repos/boostorg/spirit/issues/200","id":238435123,"user":{"login":"mxc-commons","id":5009831,"avatar_url":"https://avatars.githubusercontent.com/u/5009831?v=3","gravatar_id":"","url":"https://api.github.com/users/mxc-commons","html_url":"https://github.com/mxc-commons","followers_url":"https://api.github.com/users/mxc-commons/followers","following_url":"https://api.github.com/users/mxc-commons/following{/other_user}","gists_url":"https://api.github.com/users/mxc-commons/gists{/gist_id}","starred_url":"https://api.github.com/users/mxc-commons/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mxc-commons/subscriptions","organizations_url":"https://api.github.com/users/mxc-commons/orgs","repos_url":"https://api.github.com/users/mxc-commons/repos","events_url":"https://api.github.com/users/mxc-commons/events{/privacy}","received_events_url":"https://api.github.com/users/mxc-commons/received_events","type":"User","site_admin":false},"created_at":"2016-08-09T02:21:59Z","updated_at":"2016-08-09T02:21:59Z","body":"Thank you so much. :) As always your answer lights up my day (or night respectively ;)\r\n\r\n>I concur with @djowel : what's the hurry? If you want to establish a reputation, I believe you have already done so. (I can only wonder what kind of audience would say \"But it's not in the tar-ball release, so it doesn't count\".)\r\n\r\n>Spirit is a production library for generating parsers from a C++ eDSL. It has been for over 15 years.\r\n\r\nTo be completely honest, it's not only about reputation. Qi is important for our business. Enterprise customers nowerdays are open to Open Source OS platforms, but is still a hard  discussion to bring a dependency to a open source library in. It's less difficult, if it is labelled Boost (if you are not talking to architects and coders, and not to managers, what I usually do). If Open Source Library, then preferrably the unmodified release. Therefore my interest to contribute to the \"standard\" release. \r\n\r\n>A motivating example would help a lot. I don't disagree with the abundance of arguments you give explaining how it will complement the current EDSL expressiveness, but it's all pretty abstract. That makes it uncompelling (if we've lived without it for all these years...).\r\n\r\nIn the mailing list I introduced an example. But I'm not sure, if it's motivating enough. I took the example from our [discussion on stackoverflow](http://stackoverflow.com/questions/37669936/spirit-qi-how-can-i-write-a-nonterminal-parser](http://stackoverflow.com/questions/37669936/spirit-qi-how-can-i-write-a-nonterminal-parser). Implement a longest_match parser. I referenced the X3 solution you provided several times since then. Even in this discussion.\r\n\r\n\r\n>With the PR #201 bugfix, the qi test suite compiles with no errors or failures.\r\nThank you for that!\r\n\r\nNot for that. I propose changes and need a test suite that compiles as a minimum requirement. The PR was not accepted. Joel wants to provide a better fix himself.\r\n\r\n>Rule evaluation control: qi automatically applies rules and that works good.\r\nAnyway, there seems to be (have been) a demand by users to gain some control\r\nabout the rule evaluation control flow. Best example for that is the legendary\r\nNabialek trick which is still referenced today.\r\n>>I had trouble seeing how this relates to the PR. If I understand you correctly, I think Qi has had a few directives like this for some time. repository::qi's confix, distinct and keyword list operators come to mind. (see here).\r\n\r\n>>(Within Qi core qi::hold, qi::lazy, qi::as<>, qi::matches all act as \"higher order\" directives that control the operation of the subsidiary parsers. I suppose the major significant difference would - again - be the arity of the parser arguments a directive can take. There seem to always have been ways around that if you look at the linked Repository samples.)\r\n\r\nNone of the directives you mention intrude their operand parser's or share knowledge with them. They are all about pre-processing, post-processing, instantiation of parser which are black boxes for the \"surrounding\" directive.\r\n\r\nI guess, it's getting too abstract again. The n-ary operator proposed here handles the argument parsers as black boxes, too. But it decides what argument is executed or skipped, what attribute is taken or rejected (even if parsing of that argument parser succeeded) and so on. It implements it's own parsing logic where the argument parsers are components with hidden implementation. \r\n\r\n>So IMHO there is no risk for existing code. This assumes, that C++ is deterministic.\r\n>>Being deterministic doesn't prevent surprises. ADL, partial ordering, etc. are prime sources of such surprise. I think there have been a number of occasions in which Qi had evolved to a state where certain ill-formed/meaningless constructs would compile, with fairly arbitrary results. In most cases, these could be fixed by adding more diagnostic checks.\r\n\r\nOk. I know about the potential in general. But what can we expect of ADL, partial ordering, etc regarding surprises? If you look at this particular case statement, do you actually see room for such surprises? I took that into consideration. I can't see potential issues. What am I missing?\r\n\r\n>Another risk: If qi stops to evolve, community support will erode. That's for sure.\r\n>>That's a pretty obvious hyperbole. If Qi stops to be maintained, it will rot.\r\n\r\nTouché. :) As it is written, this is completely rhethoric.\r\n\r\nWhat I mean, Qi seems to stagnate somewhat. I don't think of new features but of maintenance. The test suite does not compile without error. The examples do not compile without error. On the other hand, there were several boost releases since the last time the tests and examples were touched. Joel urges the community, that maintenance is important. That's not enough. It's even more difficult for me to promote a library without a working test suite.  \r\n\r\n>But as it is, it is a mature library with many existing production usages. All that is required for it to stay relevant is for compiler support to be maintained while the supporting Boost libraries evolve. The evolution of variant, optional, fusion, MPL etc. have already vastly benefited Qi without requiring radical innovation to the concept of Qi.\r\n\r\n>I think stability is a core feature of most Boost libraries.\r\n\r\nOk, but very general. I do not think that all desirable evolution for qi will happen as a sideeffect of other librarie's evolution. Stability is one of the core features of boost. But there are others. If stability was the only criteria, nothing needs to happen once the thing is stable. There would not be any need for X3. So innovation and future awareness may be other important boost features. I do not see particular reason, why X3 is allowed to evolve and Qi not.\r\n\r\n\r\n>If it is true that qi is not going to get deprecated then what is important for\r\nX3 should be important for qi also. And considering the points above my answer\r\nis: Yes.\r\nI find it hard to argue that things that Qi hasn't needed for a decade are important in X3. It's just notably easier to functionally compose parsers in X3/C++14. Possible != essential.\r\n\r\nI completely agree. Possible is almost everything. But essential? I completely agree, that each feature X3 has and Qi has not has to be considered very carefully for a back-port. If a backport was only possible if Qi needed to be changed breaking it's concepts, design paradigms, etc, it should not be done. That's for sure the area where X3 will be superior to Qi with all legitimation. I would never turn Qi inside out to introduce something which is somewhat natural for X3 but almost impossible for Qi. Everything is possible, not all of that is a good idea. \r\n\r\nBut if a backport can be done maintaining philosophy, paradigms, concepts I urge that this gets considered seriously. Doing something is a risk. Not doing something is a risk also. Not everybody is aware of that. If I stand up now to get another Wodka/Lemon I could fall down the stairs. If I don't, I could die because the hardware test environment next to my place will explode and take me with. In business terms: Almost nobody is aware of the price he has to pay for non-investment. Every decision costs. Keeping the Status Quo costs. I put much efforts in this discussions. Because I believe, nobody thought about the PNI yet (the price of non-investment).\r\n\r\nThis PR backports the ability to create n-ary operators. Completely different to the X3 approach, but completely congruent with the Qi approach. \r\n\r\n>That possibility in X3 stems from simplicity.\r\n>Simplicity is essential.\r\n>What matters most, I think, is whether this PR contributes to/maintains simplicity. It seems like it does, but I'll have to find time for a closer look.\r\n\r\nIf u mean, usability I agree. Usabality for programmers in particular. AFAIK there are not much people out there who implement custom parsers. One important reason for that IMHO is that there is no documentation. The only guide how to implement a custom parser is the [iter_pos parser](http://boost-spirit.com/home/articles/qi-example/creating-your-own-parser-component-for-spirit-qi/). This article found great response but raised more questions than it answered. Life gets much easier for custom parser implementors with that PR (I mean, solving problems not currently solvable easily). How to implement a parser in general is lacking documentation topic.\r\n\r\nFrom an end user perspective, simplicity is topic as well. I believe, users are at least c++ programmers, so I am sure, that they have an immediate idea what max(a,b,c,d) does. Without that PR a complicated solution could be programmed which would end up in a user interface looking something like max[ a += b += c += d ], which would do the same as max(a,b,c,d) does with that PR. \r\n\r\nI would be happy if you would take the time to look into the code (no more than 30 lines or so (without the test)).\r\n\r\nThere are a lot of maintainance tasks open. There are even bugs open (like the single member struct problem). If the community is only asked do maintenance tasks which were obviously not important for the last years... "}}
{"actor":{"display_login":"mxc-commons"}}