{"action":"created","issue":{"url":"https://api.github.com/repos/Manu343726/snail/issues/3","labels_url":"https://api.github.com/repos/Manu343726/snail/issues/3/labels{/name}","comments_url":"https://api.github.com/repos/Manu343726/snail/issues/3/comments","events_url":"https://api.github.com/repos/Manu343726/snail/issues/3/events","html_url":"https://github.com/Manu343726/snail/issues/3","id":56533496,"number":3,"title":"SFINAE callability check, more algorithm categories, reducing allocations","user":{"login":"Manu343726","id":1561197,"avatar_url":"https://avatars.githubusercontent.com/u/1561197?v=3","gravatar_id":"","url":"https://api.github.com/users/Manu343726","html_url":"https://github.com/Manu343726","followers_url":"https://api.github.com/users/Manu343726/followers","following_url":"https://api.github.com/users/Manu343726/following{/other_user}","gists_url":"https://api.github.com/users/Manu343726/gists{/gist_id}","starred_url":"https://api.github.com/users/Manu343726/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Manu343726/subscriptions","organizations_url":"https://api.github.com/users/Manu343726/orgs","repos_url":"https://api.github.com/users/Manu343726/repos","events_url":"https://api.github.com/users/Manu343726/events{/privacy}","received_events_url":"https://api.github.com/users/Manu343726/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/Manu343726/snail/labels/enhancement","name":"enhancement","color":"84b6eb"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-02-04T14:34:17Z","updated_at":"2015-02-04T14:53:52Z","closed_at":null,"body":"[Stefan Löerwald](mailto:stefan.loerwald@gmail.com) sent me an email referencing some issues:\r\n\r\n> Hi Manuel,\r\n>\r\nI've come across your snail library and gave it a try. With gcc, I get compiler errors in the `operator|(C&&, F)`. This is due to matches to this operator with non-callable F in some internal compiler headers.\r\n\r\n>To fix this, you may use SFINAE to check if `F` is actually a callable:\r\n\r\n``` cpp\r\ntemplate <typename C, typename F>\r\nusing return_t = decltype(std::declval<F>()(std::declval<C>()));\r\n\r\ntemplate<typename C, typename F, typename = return_t<C, F>>\r\nauto operator|(C&& container, F f)\r\n{\r\n    return std::move(f(std::forward<C>(container)));\r\n}\r\n```\r\n\r\n>Another note: Constructing a new container each time is a heavy operation which may be avoided in many circumstances. Did you think of a binary_mutable_inplace? I think there should be no issue with\r\n\r\n\r\n``` cpp\r\ntemplate<>\r\nstruct algorithm_factory<snail::categories::binary_mutable_inplace>\r\n{\r\n\ttemplate<typename Algorithm>\r\n\tstatic auto make(Algorithm algorithm)\r\n\t{\r\n\t\treturn [=](auto f)\r\n\t\t{\r\n\t\t\treturn [=](auto&& container)\r\n\t\t\t{\r\n\t\t\t\tusing std::begin;\r\n\t\t\t\tusing std::end;\r\n\t\t\t\tusing std::back_inserter;\r\n\r\n\t\t\t\talgorithm( begin(container), end(container),\r\n\t\t\t\t           begin(container), f);\r\n\r\n\t\t\t\treturn std::move(container);\r\n\t\t\t};\r\n\t\t};\r\n\t}\r\n};\r\n```\r\n\r\n>and making\r\n\r\n``` cpp\r\nalgorithm_category(std::transform, snail::categories::binary_mutable_inplace)\r\n```\r\n\r\n>Of course this may not be directly applied to a filter!\r\n\r\n>For anything that supports erase, you may want to specialize the filter to.\r\n>For reducing allocations I think (not 100% sure!) you can replace the `return [=](const auto& container) by [=](auto&& container)`.\r\n\r\n>This reduces (in your example) the number of allocations from 10 to 4 (optimal would be one of course, but that's not possible with a generic back_inserter).\r\n\r\n``` cpp\r\n\ttemplate<>\r\n\tstruct algorithm_factory<snail::categories::FILTER_STUFF>\r\n\t{\r\n\t\ttemplate<typename Algorithm>\r\n\t\tstatic auto make(Algorithm algorithm)\r\n\t\t{\r\n\t\t\treturn [=](auto f)\r\n\t\t\t{\r\n\t\t\t\treturn [=](auto&& container)\r\n\t\t\t\t{\r\n\t\t\t\t\tusing std::begin;\r\n\t\t\t\t\tusing std::end;\r\n\t\t\t\t\tauto new_end = algorithm( begin(container), end(container), begin(container), f);\r\n                                        container.erase(new_end, end(container));\r\n\t\t\t\t\treturn std::move(container);\r\n\t\t\t\t};\r\n\t\t\t};\r\n\t\t}\r\n\t};\r\n```\r\n\r\n>What do you think of these changes?\r\n\r\n>Best regards,\r\n>Stefan\r\n\r\nTL;DR\r\n--------\r\n\r\nIn brief, he points out some issues the library currently has:\r\n\r\n - **The continuation does not check if the algorithm is applicable to the container**: Even if this is a rare case, having a SFINAE check there is a good thing, making compilation-errors more clear than going down until the true application, through all the factories, lambdas, etc.  \r\nBesides that, I don't understand \"With gcc, I get compiler errors in the `operator|(C&&, F)`. This is due to matches to this operator with non-callable F in some internal compiler headers\". The library is being tested on Travis CI with GCC compiler. Maybe he's using a previous version.\r\n\r\n- **`snail::categories::binary_mutable_inplace` category to reduce unnecessary container allocations**: Totally agree, adding more categories covering specific cases to improve performance is something I have to do. The current set of categories serves as a proof of concept, but it's too generic.\r\n\r\n- Using forwarding references in mutable algorithms to reduce allocations. I have to check this, I'm not sure too."},"comment":{"url":"https://api.github.com/repos/Manu343726/snail/issues/comments/72867584","html_url":"https://github.com/Manu343726/snail/issues/3#issuecomment-72867584","issue_url":"https://api.github.com/repos/Manu343726/snail/issues/3","id":72867584,"user":{"login":"stefanloerwald","id":10850250,"avatar_url":"https://avatars.githubusercontent.com/u/10850250?v=3","gravatar_id":"","url":"https://api.github.com/users/stefanloerwald","html_url":"https://github.com/stefanloerwald","followers_url":"https://api.github.com/users/stefanloerwald/followers","following_url":"https://api.github.com/users/stefanloerwald/following{/other_user}","gists_url":"https://api.github.com/users/stefanloerwald/gists{/gist_id}","starred_url":"https://api.github.com/users/stefanloerwald/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stefanloerwald/subscriptions","organizations_url":"https://api.github.com/users/stefanloerwald/orgs","repos_url":"https://api.github.com/users/stefanloerwald/repos","events_url":"https://api.github.com/users/stefanloerwald/events{/privacy}","received_events_url":"https://api.github.com/users/stefanloerwald/received_events","type":"User","site_admin":false},"created_at":"2015-02-04T14:53:52Z","updated_at":"2015-02-04T14:53:52Z","body":"The bug with gcc is certainly a nasty one. Even if it's a bug in gcc, I'd go with sfinae-enabling the method OR adding a nice and shiny static_assert."}}