{"action":"created","comment":{"url":"https://api.github.com/repos/hsutter/gcpp/pulls/comments/80391254","id":80391254,"diff_hunk":"@@ -33,59 +35,135 @@ namespace gcpp {\n \t//----------------------------------------------------------------------------\n \n \tclass bitflags {\n+\t\tusing unit = unsigned int;\n+\t\tstatic_assert(std::is_unsigned<unit>::value, \"unit must be an unsigned integral type.\");\n+\n+\t\tstd::unique_ptr<unit[]> bits;\n \t\tconst int size;\n-\t\tstd::vector<byte> bits;\n+\n+\t\tstatic constexpr auto bits_per_unit = static_cast<int>(sizeof(unit) * CHAR_BIT);\n+\n+\t\t//  Return a unit with all bits set if \"set\" is true, or all bits cleared otherwise.\n+\t\t//\n+\t\tstatic constexpr unit all_bits(bool set) {\n+\t\t\treturn set ? ~unit(0) : unit(0);\n+\t\t}\n+\n+\t\t//  Return a mask that will select the bit at position from its unit\n+\t\t//\n+\t\tstatic unit bit_mask(int at) {\n+\t\t\tExpects(0 <= at && \"position must be non-negative\");\n+\t\t\treturn unit(1) << (at % bits_per_unit);\n+\t\t}\n+\n+\t\t//  Return the number of units needed to represent a number of bits\n+\t\t//\n+\t\tstatic int unit_count(int bit_count) {\n+\t\t\tExpects(0 <= bit_count && \"bit_count must be non-negative\");\n+\t\t\treturn (bit_count + bits_per_unit - 1) / bits_per_unit;\n+\t\t}\n+\n+\t\t//  Get the unit that contains the bit at position\n+\t\t//\n+\t\tunit& bit_unit(int at) {\n+\t\t\tExpects(0 <= at && \"position must be non-negative\");\n+\t\t\treturn bits[at / bits_per_unit];\n+\t\t}\n+\t\tconst unit& bit_unit(int at) const {\n+\t\t\tExpects(0 <= at && \"position must be non-negative\");\n+\t\t\treturn bits[at / bits_per_unit];\n+\t\t}\n \n \tpublic:\n-\t\tbitflags(int bits, bool value)\n-\t\t\t: size{ bits }\n-\t\t\t, bits(1 + size / sizeof(byte), value ? byte(0xFF) : byte(0x00))\n-\t\t{ \n-\t\t\tExpects(bits > 0 && \"#bits must be positive\");\n+\t\tbitflags(int nbits, bool value)\n+\t\t\t: size{ nbits }\n+\t\t{\n+\t\t\tExpects(nbits >= 0 && \"#bits must be non-negative\");\n+\t\t\tbits = std::make_unique<unit[]>(unit_count(nbits));\n+\t\t\tif (value) {\n+\t\t\t\tset_all(true);\n+\t\t\t}\n \t\t}\n \n \t\t//\tGet flag value at position\n \t\t//\n \t\tbool get(int at) const {\n \t\t\tExpects(0 <= at && at < size && \"bitflags get() out of range\");\n-\t\t\treturn (bits[at / sizeof(byte)] & byte(1 << (at % sizeof(byte)))) > byte(0);\n+\t\t\treturn (bit_unit(at) & bit_mask(at)) != unit(0);\n \t\t}\n \n \t\t//\tSet flag value at position\n \t\t//\n \t\tvoid set(int at, bool value) {\n \t\t\tExpects(0 <= at && at < size && \"bitflags set() out of range\");\n \t\t\tif (value) {\n-\t\t\t\tbits[at / sizeof(byte)] |= byte(1 << (at % sizeof(byte)));\n+\t\t\t\tbit_unit(at) |= bit_mask(at);\n \t\t\t}","path":"bitflags.h","position":120,"original_position":120,"commit_id":"102d95a627d03baa908d7850c27874638b5c5410","original_commit_id":"102d95a627d03baa908d7850c27874638b5c5410","user":{"login":"hsutter","id":1801526,"avatar_url":"https://avatars.githubusercontent.com/u/1801526?v=3","gravatar_id":"","url":"https://api.github.com/users/hsutter","html_url":"https://github.com/hsutter","followers_url":"https://api.github.com/users/hsutter/followers","following_url":"https://api.github.com/users/hsutter/following{/other_user}","gists_url":"https://api.github.com/users/hsutter/gists{/gist_id}","starred_url":"https://api.github.com/users/hsutter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hsutter/subscriptions","organizations_url":"https://api.github.com/users/hsutter/orgs","repos_url":"https://api.github.com/users/hsutter/repos","events_url":"https://api.github.com/users/hsutter/events{/privacy}","received_events_url":"https://api.github.com/users/hsutter/received_events","type":"User","site_admin":false},"body":"I like this elegance. At first I wondered why `bit_mask` did modulus instead of enforcing that `at` was less than `bits_per_unit`, but now I see you did that to support this usage. Thanks.","created_at":"2016-09-25T20:31:26Z","updated_at":"2016-09-25T20:31:26Z","html_url":"https://github.com/hsutter/gcpp/pull/9#discussion_r80391254","pull_request_url":"https://api.github.com/repos/hsutter/gcpp/pulls/9","_links":{"self":{"href":"https://api.github.com/repos/hsutter/gcpp/pulls/comments/80391254"},"html":{"href":"https://github.com/hsutter/gcpp/pull/9#discussion_r80391254"},"pull_request":{"href":"https://api.github.com/repos/hsutter/gcpp/pulls/9"}}},"pull_request":{"url":"https://api.github.com/repos/hsutter/gcpp/pulls/9","id":86637635,"html_url":"https://github.com/hsutter/gcpp/pull/9","diff_url":"https://github.com/hsutter/gcpp/pull/9.diff","patch_url":"https://github.com/hsutter/gcpp/pull/9.patch","issue_url":"https://api.github.com/repos/hsutter/gcpp/issues/9","number":9,"state":"open","locked":false,"title":"bitflags: several changes","user":{"login":"CaseyCarter","id":456873,"avatar_url":"https://avatars.githubusercontent.com/u/456873?v=3","gravatar_id":"","url":"https://api.github.com/users/CaseyCarter","html_url":"https://github.com/CaseyCarter","followers_url":"https://api.github.com/users/CaseyCarter/followers","following_url":"https://api.github.com/users/CaseyCarter/following{/other_user}","gists_url":"https://api.github.com/users/CaseyCarter/gists{/gist_id}","starred_url":"https://api.github.com/users/CaseyCarter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CaseyCarter/subscriptions","organizations_url":"https://api.github.com/users/CaseyCarter/orgs","repos_url":"https://api.github.com/users/CaseyCarter/repos","events_url":"https://api.github.com/users/CaseyCarter/events{/privacy}","received_events_url":"https://api.github.com/users/CaseyCarter/received_events","type":"User","site_admin":false},"body":"* `byte` is an odd choice for the units of a bit vector: we care that it's a bag of bits, there's no reason to use the \"fundamental unit of addressable storage\" type. Instead use the symbolic type `unit`, currently a `typedef` to `unsigned int` so `set(from, to, value)` can process bigger chunks.\r\n\r\n* `sizeof(byte)` is not the number of bits in a unit; it is a complicated spelling of `1`. `bitflags` is therefore `CHAR_BIT` times as large as it needs to be. Replace uses of `sizeof(byte)` with `bits_per_unit` which is `sizeof(unit) * CHAR_BIT`.\r\n\r\n* Factor the complex \"bit addressing\" expressions into separate functions to simplify.\r\n\r\n* Use `std::unique_ptr<unit[]>` for bit storage instead of `std::vector<unit>`; some guy said in a talk at CPPCon that we're supposed to use `std::unique_ptr<T[]>` to \"express a fixed-but-dynamic-size member array\" ;)\r\n\r\n* Improve codegen for `set(from, to, value)` by implementing the head and tail loops in O(1) with bit shift/mask operations.\r\n","created_at":"2016-09-25T02:47:20Z","updated_at":"2016-09-25T20:31:26Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b6c24400a402c6f00e7c83363f6ef7c110c2fa4c","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/hsutter/gcpp/pulls/9/commits","review_comments_url":"https://api.github.com/repos/hsutter/gcpp/pulls/9/comments","review_comment_url":"https://api.github.com/repos/hsutter/gcpp/pulls/comments{/number}","comments_url":"https://api.github.com/repos/hsutter/gcpp/issues/9/comments","statuses_url":"https://api.github.com/repos/hsutter/gcpp/statuses/102d95a627d03baa908d7850c27874638b5c5410","head":{"label":"CaseyCarter:bitflags_rework","ref":"bitflags_rework","sha":"102d95a627d03baa908d7850c27874638b5c5410","user":{"login":"CaseyCarter","id":456873,"avatar_url":"https://avatars.githubusercontent.com/u/456873?v=3","gravatar_id":"","url":"https://api.github.com/users/CaseyCarter","html_url":"https://github.com/CaseyCarter","followers_url":"https://api.github.com/users/CaseyCarter/followers","following_url":"https://api.github.com/users/CaseyCarter/following{/other_user}","gists_url":"https://api.github.com/users/CaseyCarter/gists{/gist_id}","starred_url":"https://api.github.com/users/CaseyCarter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CaseyCarter/subscriptions","organizations_url":"https://api.github.com/users/CaseyCarter/orgs","repos_url":"https://api.github.com/users/CaseyCarter/repos","events_url":"https://api.github.com/users/CaseyCarter/events{/privacy}","received_events_url":"https://api.github.com/users/CaseyCarter/received_events","type":"User","site_admin":false},"repo":{"id":69081260,"name":"gcpp","full_name":"CaseyCarter/gcpp","owner":{"login":"CaseyCarter","id":456873,"avatar_url":"https://avatars.githubusercontent.com/u/456873?v=3","gravatar_id":"","url":"https://api.github.com/users/CaseyCarter","html_url":"https://github.com/CaseyCarter","followers_url":"https://api.github.com/users/CaseyCarter/followers","following_url":"https://api.github.com/users/CaseyCarter/following{/other_user}","gists_url":"https://api.github.com/users/CaseyCarter/gists{/gist_id}","starred_url":"https://api.github.com/users/CaseyCarter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CaseyCarter/subscriptions","organizations_url":"https://api.github.com/users/CaseyCarter/orgs","repos_url":"https://api.github.com/users/CaseyCarter/repos","events_url":"https://api.github.com/users/CaseyCarter/events{/privacy}","received_events_url":"https://api.github.com/users/CaseyCarter/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/CaseyCarter/gcpp","description":"Experimental deferred and unordered destruction library for C++","fork":true,"url":"https://api.github.com/repos/CaseyCarter/gcpp","forks_url":"https://api.github.com/repos/CaseyCarter/gcpp/forks","keys_url":"https://api.github.com/repos/CaseyCarter/gcpp/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CaseyCarter/gcpp/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CaseyCarter/gcpp/teams","hooks_url":"https://api.github.com/repos/CaseyCarter/gcpp/hooks","issue_events_url":"https://api.github.com/repos/CaseyCarter/gcpp/issues/events{/number}","events_url":"https://api.github.com/repos/CaseyCarter/gcpp/events","assignees_url":"https://api.github.com/repos/CaseyCarter/gcpp/assignees{/user}","branches_url":"https://api.github.com/repos/CaseyCarter/gcpp/branches{/branch}","tags_url":"https://api.github.com/repos/CaseyCarter/gcpp/tags","blobs_url":"https://api.github.com/repos/CaseyCarter/gcpp/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CaseyCarter/gcpp/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CaseyCarter/gcpp/git/refs{/sha}","trees_url":"https://api.github.com/repos/CaseyCarter/gcpp/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CaseyCarter/gcpp/statuses/{sha}","languages_url":"https://api.github.com/repos/CaseyCarter/gcpp/languages","stargazers_url":"https://api.github.com/repos/CaseyCarter/gcpp/stargazers","contributors_url":"https://api.github.com/repos/CaseyCarter/gcpp/contributors","subscribers_url":"https://api.github.com/repos/CaseyCarter/gcpp/subscribers","subscription_url":"https://api.github.com/repos/CaseyCarter/gcpp/subscription","commits_url":"https://api.github.com/repos/CaseyCarter/gcpp/commits{/sha}","git_commits_url":"https://api.github.com/repos/CaseyCarter/gcpp/git/commits{/sha}","comments_url":"https://api.github.com/repos/CaseyCarter/gcpp/comments{/number}","issue_comment_url":"https://api.github.com/repos/CaseyCarter/gcpp/issues/comments{/number}","contents_url":"https://api.github.com/repos/CaseyCarter/gcpp/contents/{+path}","compare_url":"https://api.github.com/repos/CaseyCarter/gcpp/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CaseyCarter/gcpp/merges","archive_url":"https://api.github.com/repos/CaseyCarter/gcpp/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CaseyCarter/gcpp/downloads","issues_url":"https://api.github.com/repos/CaseyCarter/gcpp/issues{/number}","pulls_url":"https://api.github.com/repos/CaseyCarter/gcpp/pulls{/number}","milestones_url":"https://api.github.com/repos/CaseyCarter/gcpp/milestones{/number}","notifications_url":"https://api.github.com/repos/CaseyCarter/gcpp/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CaseyCarter/gcpp/labels{/name}","releases_url":"https://api.github.com/repos/CaseyCarter/gcpp/releases{/id}","deployments_url":"https://api.github.com/repos/CaseyCarter/gcpp/deployments","created_at":"2016-09-24T05:02:26Z","updated_at":"2016-09-24T05:02:28Z","pushed_at":"2016-09-25T18:44:29Z","git_url":"git://github.com/CaseyCarter/gcpp.git","ssh_url":"git@github.com:CaseyCarter/gcpp.git","clone_url":"https://github.com/CaseyCarter/gcpp.git","svn_url":"https://github.com/CaseyCarter/gcpp","homepage":"","size":183,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"hsutter:master","ref":"master","sha":"7b8f0f424405bdc9315e0601f56db34a04fdc1e3","user":{"login":"hsutter","id":1801526,"avatar_url":"https://avatars.githubusercontent.com/u/1801526?v=3","gravatar_id":"","url":"https://api.github.com/users/hsutter","html_url":"https://github.com/hsutter","followers_url":"https://api.github.com/users/hsutter/followers","following_url":"https://api.github.com/users/hsutter/following{/other_user}","gists_url":"https://api.github.com/users/hsutter/gists{/gist_id}","starred_url":"https://api.github.com/users/hsutter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hsutter/subscriptions","organizations_url":"https://api.github.com/users/hsutter/orgs","repos_url":"https://api.github.com/users/hsutter/repos","events_url":"https://api.github.com/users/hsutter/events{/privacy}","received_events_url":"https://api.github.com/users/hsutter/received_events","type":"User","site_admin":false},"repo":{"id":67135324,"name":"gcpp","full_name":"hsutter/gcpp","owner":{"login":"hsutter","id":1801526,"avatar_url":"https://avatars.githubusercontent.com/u/1801526?v=3","gravatar_id":"","url":"https://api.github.com/users/hsutter","html_url":"https://github.com/hsutter","followers_url":"https://api.github.com/users/hsutter/followers","following_url":"https://api.github.com/users/hsutter/following{/other_user}","gists_url":"https://api.github.com/users/hsutter/gists{/gist_id}","starred_url":"https://api.github.com/users/hsutter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hsutter/subscriptions","organizations_url":"https://api.github.com/users/hsutter/orgs","repos_url":"https://api.github.com/users/hsutter/repos","events_url":"https://api.github.com/users/hsutter/events{/privacy}","received_events_url":"https://api.github.com/users/hsutter/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/hsutter/gcpp","description":"Experimental deferred and unordered destruction library for C++","fork":false,"url":"https://api.github.com/repos/hsutter/gcpp","forks_url":"https://api.github.com/repos/hsutter/gcpp/forks","keys_url":"https://api.github.com/repos/hsutter/gcpp/keys{/key_id}","collaborators_url":"https://api.github.com/repos/hsutter/gcpp/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/hsutter/gcpp/teams","hooks_url":"https://api.github.com/repos/hsutter/gcpp/hooks","issue_events_url":"https://api.github.com/repos/hsutter/gcpp/issues/events{/number}","events_url":"https://api.github.com/repos/hsutter/gcpp/events","assignees_url":"https://api.github.com/repos/hsutter/gcpp/assignees{/user}","branches_url":"https://api.github.com/repos/hsutter/gcpp/branches{/branch}","tags_url":"https://api.github.com/repos/hsutter/gcpp/tags","blobs_url":"https://api.github.com/repos/hsutter/gcpp/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/hsutter/gcpp/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/hsutter/gcpp/git/refs{/sha}","trees_url":"https://api.github.com/repos/hsutter/gcpp/git/trees{/sha}","statuses_url":"https://api.github.com/repos/hsutter/gcpp/statuses/{sha}","languages_url":"https://api.github.com/repos/hsutter/gcpp/languages","stargazers_url":"https://api.github.com/repos/hsutter/gcpp/stargazers","contributors_url":"https://api.github.com/repos/hsutter/gcpp/contributors","subscribers_url":"https://api.github.com/repos/hsutter/gcpp/subscribers","subscription_url":"https://api.github.com/repos/hsutter/gcpp/subscription","commits_url":"https://api.github.com/repos/hsutter/gcpp/commits{/sha}","git_commits_url":"https://api.github.com/repos/hsutter/gcpp/git/commits{/sha}","comments_url":"https://api.github.com/repos/hsutter/gcpp/comments{/number}","issue_comment_url":"https://api.github.com/repos/hsutter/gcpp/issues/comments{/number}","contents_url":"https://api.github.com/repos/hsutter/gcpp/contents/{+path}","compare_url":"https://api.github.com/repos/hsutter/gcpp/compare/{base}...{head}","merges_url":"https://api.github.com/repos/hsutter/gcpp/merges","archive_url":"https://api.github.com/repos/hsutter/gcpp/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/hsutter/gcpp/downloads","issues_url":"https://api.github.com/repos/hsutter/gcpp/issues{/number}","pulls_url":"https://api.github.com/repos/hsutter/gcpp/pulls{/number}","milestones_url":"https://api.github.com/repos/hsutter/gcpp/milestones{/number}","notifications_url":"https://api.github.com/repos/hsutter/gcpp/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/hsutter/gcpp/labels{/name}","releases_url":"https://api.github.com/repos/hsutter/gcpp/releases{/id}","deployments_url":"https://api.github.com/repos/hsutter/gcpp/deployments","created_at":"2016-09-01T13:50:11Z","updated_at":"2016-09-25T20:22:15Z","pushed_at":"2016-09-25T18:44:29Z","git_url":"git://github.com/hsutter/gcpp.git","ssh_url":"git@github.com:hsutter/gcpp.git","clone_url":"https://github.com/hsutter/gcpp.git","svn_url":"https://github.com/hsutter/gcpp","homepage":"","size":196,"stargazers_count":201,"watchers_count":201,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":10,"mirror_url":null,"open_issues_count":1,"forks":10,"open_issues":1,"watchers":201,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/hsutter/gcpp/pulls/9"},"html":{"href":"https://github.com/hsutter/gcpp/pull/9"},"issue":{"href":"https://api.github.com/repos/hsutter/gcpp/issues/9"},"comments":{"href":"https://api.github.com/repos/hsutter/gcpp/issues/9/comments"},"review_comments":{"href":"https://api.github.com/repos/hsutter/gcpp/pulls/9/comments"},"review_comment":{"href":"https://api.github.com/repos/hsutter/gcpp/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/hsutter/gcpp/pulls/9/commits"},"statuses":{"href":"https://api.github.com/repos/hsutter/gcpp/statuses/102d95a627d03baa908d7850c27874638b5c5410"}}}}
{"actor":{"display_login":"hsutter"}}