{"action":"created","issue":{"url":"https://api.github.com/repos/dlang/phobos/issues/4981","repository_url":"https://api.github.com/repos/dlang/phobos","labels_url":"https://api.github.com/repos/dlang/phobos/issues/4981/labels{/name}","comments_url":"https://api.github.com/repos/dlang/phobos/issues/4981/comments","events_url":"https://api.github.com/repos/dlang/phobos/issues/4981/events","html_url":"https://github.com/dlang/phobos/pull/4981","id":196903718,"number":4981,"title":"First cleanup of bitwise adapter + add to docs","user":{"login":"wilzbach","id":4370550,"avatar_url":"https://avatars.githubusercontent.com/u/4370550?v=3","gravatar_id":"","url":"https://api.github.com/users/wilzbach","html_url":"https://github.com/wilzbach","followers_url":"https://api.github.com/users/wilzbach/followers","following_url":"https://api.github.com/users/wilzbach/following{/other_user}","gists_url":"https://api.github.com/users/wilzbach/gists{/gist_id}","starred_url":"https://api.github.com/users/wilzbach/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wilzbach/subscriptions","organizations_url":"https://api.github.com/users/wilzbach/orgs","repos_url":"https://api.github.com/users/wilzbach/repos","events_url":"https://api.github.com/users/wilzbach/events{/privacy}","received_events_url":"https://api.github.com/users/wilzbach/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/dlang/phobos/milestones/11","html_url":"https://github.com/dlang/phobos/milestone/11","labels_url":"https://api.github.com/repos/dlang/phobos/milestones/11/labels","id":2192401,"number":11,"title":"2.073.0-b0","description":null,"creator":{"login":"MartinNowak","id":288976,"avatar_url":"https://avatars.githubusercontent.com/u/288976?v=3","gravatar_id":"","url":"https://api.github.com/users/MartinNowak","html_url":"https://github.com/MartinNowak","followers_url":"https://api.github.com/users/MartinNowak/followers","following_url":"https://api.github.com/users/MartinNowak/following{/other_user}","gists_url":"https://api.github.com/users/MartinNowak/gists{/gist_id}","starred_url":"https://api.github.com/users/MartinNowak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MartinNowak/subscriptions","organizations_url":"https://api.github.com/users/MartinNowak/orgs","repos_url":"https://api.github.com/users/MartinNowak/repos","events_url":"https://api.github.com/users/MartinNowak/events{/privacy}","received_events_url":"https://api.github.com/users/MartinNowak/received_events","type":"User","site_admin":false},"open_issues":3,"closed_issues":1,"state":"open","created_at":"2016-12-12T23:23:59Z","updated_at":"2016-12-21T18:41:09Z","due_on":"2017-01-04T08:00:00Z","closed_at":null},"comments":4,"created_at":"2016-12-21T10:58:23Z","updated_at":"2016-12-23T20:11:27Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/dlang/phobos/pulls/4981","html_url":"https://github.com/dlang/phobos/pull/4981","diff_url":"https://github.com/dlang/phobos/pull/4981.diff","patch_url":"https://github.com/dlang/phobos/pull/4981.patch"},"body":"Follow-up to @edi33416's https://github.com/dlang/phobos/pull/4927 (which imho was merged too early) and adds some of my unaddressed review comments:\r\n\r\n- adds to booktable (s.t.  it's listed on the [docs](http://dlang.org/phobos-prerelease/std_range.html)\r\n- adds `bitwise` and hides the helper struct (no need to expose it and gives more flexibility later. The exposed public symbols are a relict)\r\n- adds a nice public example and hides the internal tests (see [current docs](http://dlang.org/phobos-prerelease/std_range.html#.Bitwise) for the motivation\r\n- fixes style (no parenthesis for `@property` functions)\r\n- fixed various typos\r\n- removes `__PRETTY_FUNCTION__` from `assert` (current Phobos policy is to use plain `assert`. Improvements to `assert` should be done in the compiler)\r\n- addresses @andralex's remaining comment\r\n\r\n@edi33416 could you please add _a lot more_ tests?\r\nWhen I tried to come up with nice examples for the documentation I immediately hit a bug:\r\n\r\n```\r\n[3, 9].bitwise[$ - 5 .. $ - 1].writeln; // ERROR: Attempting to fetch the front of an empty array of int\r\n```\r\n\r\nAlso please:\r\n\r\n1) See my question about calculating the mask every time in `front`.\r\n\r\n> Calculating the mask every time seems to be an unnecessary expensive operation. What is your rationale for storing the position and not the mask directly?\r\n(`front` is expected to be called a lot more often than`opSlice and `opIndex`)\r\n\r\n2) See Andrei's remark about uncovered lines\r\n\r\n3) Add a `@nogc` `unittest`\r\n\r\n4) Make the second `unittest` `@safe` as well \r\n\r\n5) Maybe add more \"showcase\" examples if you wish so\r\n\r\n6) Don't forget to add a changelog as well\r\n\r\n7) Did you do some benchmarks on iteration via bitwise and \"naive\" bitwise iteration? (i.e. the incurred overhead)"},"comment":{"url":"https://api.github.com/repos/dlang/phobos/issues/comments/269040398","html_url":"https://github.com/dlang/phobos/pull/4981#issuecomment-269040398","issue_url":"https://api.github.com/repos/dlang/phobos/issues/4981","id":269040398,"user":{"login":"wilzbach","id":4370550,"avatar_url":"https://avatars.githubusercontent.com/u/4370550?v=3","gravatar_id":"","url":"https://api.github.com/users/wilzbach","html_url":"https://github.com/wilzbach","followers_url":"https://api.github.com/users/wilzbach/followers","following_url":"https://api.github.com/users/wilzbach/following{/other_user}","gists_url":"https://api.github.com/users/wilzbach/gists{/gist_id}","starred_url":"https://api.github.com/users/wilzbach/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wilzbach/subscriptions","organizations_url":"https://api.github.com/users/wilzbach/orgs","repos_url":"https://api.github.com/users/wilzbach/repos","events_url":"https://api.github.com/users/wilzbach/events{/privacy}","received_events_url":"https://api.github.com/users/wilzbach/received_events","type":"User","site_admin":false},"created_at":"2016-12-23T20:11:27Z","updated_at":"2016-12-23T20:11:27Z","body":"> Andrei pointed out during the first stages of the review that storing both the mask and the maskPos is redundant since we can calculate one based on the other.\r\n\r\nYes, but my question was whether you benchmarked before you choose to store `maskPos`?\r\nIt's just that for e.g. `front` (a) it looks like a lot of work to do `parent.front & mask(maskPos)` vs. `parent.front & mask`\r\n\r\n>> Did you do some benchmarks on iteration via bitwise and \"naive\" bitwise iteration? (i.e. the incurred overhead)\r\n> No, I have not.\r\n\r\nIt would be great if you could do so.\r\nYou can use e.g. [benchmark](http://dlang.org/phobos/std_datetime.html#.benchmark).\r\nPeople usually use bit* stuff if they want superior performance.\r\n\r\nBtw @edi33416 you know about [core.bitop](https://dlang.org/phobos/core_bitop.html), right?\r\n(these function are intrinsics and guaranteed to be one assembly instruction - well at least if your architecture supports them) "}}
{"actor":{"display_login":"wilzbach"}}