{"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":40,"created_at":"2016-07-27T23:21:05Z","updated_at":"2016-08-11T21:49:28Z","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/239303896","html_url":"https://github.com/boostorg/spirit/pull/200#issuecomment-239303896","issue_url":"https://api.github.com/repos/boostorg/spirit/issues/200","id":239303896,"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-11T21:49:27Z","updated_at":"2016-08-11T21:49:27Z","body":">>So `if_(cond)[ a[fa] ].else_[ b[fb] ] [f_if]` would have to be, too?\r\n>\r\n>Yes, and yes.\r\n\r\nAnd cond could be a parser also?"}}
{"actor":{"display_login":"mxc-commons"}}