{"action":"created","comment":{"url":"https://api.github.com/repos/softdevteam/grmtools/pulls/comments/209207836","pull_request_review_id":145177119,"id":209207836,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIwOTIwNzgzNg==","diff_hunk":"@@ -101,14 +104,33 @@ pub struct YaccGrammar {\n // Internally, we assume that a grammar's start rule has a single production. Since we manually\n // create the start rule ourselves (without relying on user input), this is a safe assumption.\n \n-impl YaccGrammar {\n-    /// Translate a `GrammarAST` into a `YaccGrammar`. This function is akin to the part a traditional\n-    /// compiler that takes in an AST and converts it into a binary.\n+impl YaccGrammar<u32> {\n+    pub fn new(yacc_kind: YaccKind, s: &str) -> Result<Self, YaccGrammarError> {\n+        YaccGrammar::new_with_storaget(yacc_kind, s)\n+    }\n+}\n+\n+impl<StorageT: PrimInt + Unsigned> YaccGrammar<StorageT> {\n+    /// Takes as input a Yacc grammar of [`YaccKind`](enum.YaccKind.html) as a `String` `s` and returns a\n+    /// [`YaccGrammar`](grammar/struct.YaccGrammar.html) (or\n+    /// ([`YaccGrammarError`](grammar/enum.YaccGrammarError.html) on error).\n     ///\n-    /// As we're compiling the `GrammarAST` into a `Grammar` we add a new start rule (which we'll\n-    /// refer to as `^`, though the actual name is a fresh name that is guaranteed to be unique)\n-    /// that references the user defined start rule.\n-    pub fn new(yacc_kind: YaccKind, ast: &ast::GrammarAST) -> YaccGrammar {\n+    /// As we're compiling the `YaccGrammar`, we add a new start rule (which we'll refer to as `^`,\n+    /// though the actual name is a fresh name that is guaranteed to be unique) that references the\n+    /// user defined start rule.\n+    pub fn new_with_storaget(yacc_kind: YaccKind, s: &str)\n+                          -> Result<Self, YaccGrammarError>\n+    {\n+        let ast = match yacc_kind {\n+            YaccKind::Original | YaccKind::Eco => {","path":"cfgrammar/src/lib/yacc/grammar.rs","position":79,"original_position":66,"commit_id":"2a47105cb894c1abb656db8ac707a5f724d9e350","original_commit_id":"ea7fc39976587253aa3496f05556a27ee39a5f5e","user":{"login":"snim2","id":97674,"node_id":"MDQ6VXNlcjk3Njc0","avatar_url":"https://avatars2.githubusercontent.com/u/97674?v=4","gravatar_id":"","url":"https://api.github.com/users/snim2","html_url":"https://github.com/snim2","followers_url":"https://api.github.com/users/snim2/followers","following_url":"https://api.github.com/users/snim2/following{/other_user}","gists_url":"https://api.github.com/users/snim2/gists{/gist_id}","starred_url":"https://api.github.com/users/snim2/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/snim2/subscriptions","organizations_url":"https://api.github.com/users/snim2/orgs","repos_url":"https://api.github.com/users/snim2/repos","events_url":"https://api.github.com/users/snim2/events{/privacy}","received_events_url":"https://api.github.com/users/snim2/received_events","type":"User","site_admin":false},"body":"OK","created_at":"2018-08-10T09:55:44Z","updated_at":"2018-08-10T09:55:44Z","html_url":"https://github.com/softdevteam/grmtools/pull/2#discussion_r209207836","pull_request_url":"https://api.github.com/repos/softdevteam/grmtools/pulls/2","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/softdevteam/grmtools/pulls/comments/209207836"},"html":{"href":"https://github.com/softdevteam/grmtools/pull/2#discussion_r209207836"},"pull_request":{"href":"https://api.github.com/repos/softdevteam/grmtools/pulls/2"}},"in_reply_to_id":209205407},"pull_request":{"url":"https://api.github.com/repos/softdevteam/grmtools/pulls/2","id":207442080,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA3NDQyMDgw","html_url":"https://github.com/softdevteam/grmtools/pull/2","diff_url":"https://github.com/softdevteam/grmtools/pull/2.diff","patch_url":"https://github.com/softdevteam/grmtools/pull/2.patch","issue_url":"https://api.github.com/repos/softdevteam/grmtools/issues/2","number":2,"state":"open","locked":false,"title":"Better newtypes","user":{"login":"ltratt","id":20318,"node_id":"MDQ6VXNlcjIwMzE4","avatar_url":"https://avatars1.githubusercontent.com/u/20318?v=4","gravatar_id":"","url":"https://api.github.com/users/ltratt","html_url":"https://github.com/ltratt","followers_url":"https://api.github.com/users/ltratt/followers","following_url":"https://api.github.com/users/ltratt/following{/other_user}","gists_url":"https://api.github.com/users/ltratt/gists{/gist_id}","starred_url":"https://api.github.com/users/ltratt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ltratt/subscriptions","organizations_url":"https://api.github.com/users/ltratt/orgs","repos_url":"https://api.github.com/users/ltratt/repos","events_url":"https://api.github.com/users/ltratt/events{/privacy}","received_events_url":"https://api.github.com/users/ltratt/received_events","type":"User","site_admin":false},"body":"This is a scary looking PR, but its basic idea is simple: to use newtypes in a better way in grmtools. Previously we had a struct that looked roughly like this:\r\n\r\n```rust\r\nstruct NTIdx { v: u16 }\r\n```\r\n\r\nwhich then exposed an API which made it look like `NTIdx` was storing `u32`s (this was my attempt at future proofing). It was a bad idea, and made the code a lot more convoluted. This PR moves to using a tuple struct with a user specified storage size, along the lines of:\r\n\r\n```rust\r\nstruct NTIdx<StorageT> (StorageT)\r\n```\r\n\r\nThis is much easier to work with, and allows the user to choose the storage size that works best for them. [If they don't choose, things default to `u32` which should be big enough for even the craziest of crazy grammars -- if someone has more than 4294967295 productions in a grammar, they probably have lots of other problems.] It will also brings better static type checking and will make the compile-time generation of constants (using one of the `builder.rs` APIs) possible without forcing the user to pull in additional dependencies.\r\n\r\nThe basic motivation is explained in https://github.com/softdevteam/grmtools/commit/ea7fc39976587253aa3496f05556a27ee39a5f5e: that commit is also, unfortunately, borderline unreviewable, because so many changes are needed to make the system compile given the changes required (threading `StorageT` through causes ripples throughout the system).\r\n\r\nThe several following commits (https://github.com/softdevteam/grmtools/commit/a8a4b54a18b4709f4465e8841539c572e77f8ef1, https://github.com/softdevteam/grmtools/commit/ae799bc02d54524073eb1c6eadb63358e6e3c2ca, https://github.com/softdevteam/grmtools/commit/be9ffe0d7b3f76488dcfda6d2eee21e860bc04a4) are in the same vein but have a much easier job, because https://github.com/softdevteam/grmtools/commit/ea7fc39976587253aa3496f05556a27ee39a5f5e provides most (not all, but most) of the necessary infrastructure.\r\n\r\nThere are then a few tidy-up commits at the end.\r\n\r\nI think this PR is useful as it stands, and you can get a little taste of that in https://github.com/softdevteam/grmtools/commit/536a889ca3339d6643ad460a6083a6418d5698e8, but I hope for more to come later. However, this PR is big enough already, so I think it's best to try and review this (as best we can), and do more stuff in later PRs.","created_at":"2018-08-09T20:59:19Z","updated_at":"2018-08-10T09:55:44Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d902fd6695162067342fac90977cfaff735de668","assignee":{"login":"snim2","id":97674,"node_id":"MDQ6VXNlcjk3Njc0","avatar_url":"https://avatars2.githubusercontent.com/u/97674?v=4","gravatar_id":"","url":"https://api.github.com/users/snim2","html_url":"https://github.com/snim2","followers_url":"https://api.github.com/users/snim2/followers","following_url":"https://api.github.com/users/snim2/following{/other_user}","gists_url":"https://api.github.com/users/snim2/gists{/gist_id}","starred_url":"https://api.github.com/users/snim2/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/snim2/subscriptions","organizations_url":"https://api.github.com/users/snim2/orgs","repos_url":"https://api.github.com/users/snim2/repos","events_url":"https://api.github.com/users/snim2/events{/privacy}","received_events_url":"https://api.github.com/users/snim2/received_events","type":"User","site_admin":false},"assignees":[{"login":"snim2","id":97674,"node_id":"MDQ6VXNlcjk3Njc0","avatar_url":"https://avatars2.githubusercontent.com/u/97674?v=4","gravatar_id":"","url":"https://api.github.com/users/snim2","html_url":"https://github.com/snim2","followers_url":"https://api.github.com/users/snim2/followers","following_url":"https://api.github.com/users/snim2/following{/other_user}","gists_url":"https://api.github.com/users/snim2/gists{/gist_id}","starred_url":"https://api.github.com/users/snim2/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/snim2/subscriptions","organizations_url":"https://api.github.com/users/snim2/orgs","repos_url":"https://api.github.com/users/snim2/repos","events_url":"https://api.github.com/users/snim2/events{/privacy}","received_events_url":"https://api.github.com/users/snim2/received_events","type":"User","site_admin":false}],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/softdevteam/grmtools/pulls/2/commits","review_comments_url":"https://api.github.com/repos/softdevteam/grmtools/pulls/2/comments","review_comment_url":"https://api.github.com/repos/softdevteam/grmtools/pulls/comments{/number}","comments_url":"https://api.github.com/repos/softdevteam/grmtools/issues/2/comments","statuses_url":"https://api.github.com/repos/softdevteam/grmtools/statuses/2a47105cb894c1abb656db8ac707a5f724d9e350","head":{"label":"softdevteam:flexible_newtypes","ref":"flexible_newtypes","sha":"2a47105cb894c1abb656db8ac707a5f724d9e350","user":{"login":"softdevteam","id":10271085,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwMjcxMDg1","avatar_url":"https://avatars0.githubusercontent.com/u/10271085?v=4","gravatar_id":"","url":"https://api.github.com/users/softdevteam","html_url":"https://github.com/softdevteam","followers_url":"https://api.github.com/users/softdevteam/followers","following_url":"https://api.github.com/users/softdevteam/following{/other_user}","gists_url":"https://api.github.com/users/softdevteam/gists{/gist_id}","starred_url":"https://api.github.com/users/softdevteam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/softdevteam/subscriptions","organizations_url":"https://api.github.com/users/softdevteam/orgs","repos_url":"https://api.github.com/users/softdevteam/repos","events_url":"https://api.github.com/users/softdevteam/events{/privacy}","received_events_url":"https://api.github.com/users/softdevteam/received_events","type":"Organization","site_admin":false},"repo":{"id":143743108,"node_id":"MDEwOlJlcG9zaXRvcnkxNDM3NDMxMDg=","name":"grmtools","full_name":"softdevteam/grmtools","owner":{"login":"softdevteam","id":10271085,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwMjcxMDg1","avatar_url":"https://avatars0.githubusercontent.com/u/10271085?v=4","gravatar_id":"","url":"https://api.github.com/users/softdevteam","html_url":"https://github.com/softdevteam","followers_url":"https://api.github.com/users/softdevteam/followers","following_url":"https://api.github.com/users/softdevteam/following{/other_user}","gists_url":"https://api.github.com/users/softdevteam/gists{/gist_id}","starred_url":"https://api.github.com/users/softdevteam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/softdevteam/subscriptions","organizations_url":"https://api.github.com/users/softdevteam/orgs","repos_url":"https://api.github.com/users/softdevteam/repos","events_url":"https://api.github.com/users/softdevteam/events{/privacy}","received_events_url":"https://api.github.com/users/softdevteam/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/softdevteam/grmtools","description":"Rust grammar tool libraries and binaries","fork":false,"url":"https://api.github.com/repos/softdevteam/grmtools","forks_url":"https://api.github.com/repos/softdevteam/grmtools/forks","keys_url":"https://api.github.com/repos/softdevteam/grmtools/keys{/key_id}","collaborators_url":"https://api.github.com/repos/softdevteam/grmtools/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/softdevteam/grmtools/teams","hooks_url":"https://api.github.com/repos/softdevteam/grmtools/hooks","issue_events_url":"https://api.github.com/repos/softdevteam/grmtools/issues/events{/number}","events_url":"https://api.github.com/repos/softdevteam/grmtools/events","assignees_url":"https://api.github.com/repos/softdevteam/grmtools/assignees{/user}","branches_url":"https://api.github.com/repos/softdevteam/grmtools/branches{/branch}","tags_url":"https://api.github.com/repos/softdevteam/grmtools/tags","blobs_url":"https://api.github.com/repos/softdevteam/grmtools/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/softdevteam/grmtools/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/softdevteam/grmtools/git/refs{/sha}","trees_url":"https://api.github.com/repos/softdevteam/grmtools/git/trees{/sha}","statuses_url":"https://api.github.com/repos/softdevteam/grmtools/statuses/{sha}","languages_url":"https://api.github.com/repos/softdevteam/grmtools/languages","stargazers_url":"https://api.github.com/repos/softdevteam/grmtools/stargazers","contributors_url":"https://api.github.com/repos/softdevteam/grmtools/contributors","subscribers_url":"https://api.github.com/repos/softdevteam/grmtools/subscribers","subscription_url":"https://api.github.com/repos/softdevteam/grmtools/subscription","commits_url":"https://api.github.com/repos/softdevteam/grmtools/commits{/sha}","git_commits_url":"https://api.github.com/repos/softdevteam/grmtools/git/commits{/sha}","comments_url":"https://api.github.com/repos/softdevteam/grmtools/comments{/number}","issue_comment_url":"https://api.github.com/repos/softdevteam/grmtools/issues/comments{/number}","contents_url":"https://api.github.com/repos/softdevteam/grmtools/contents/{+path}","compare_url":"https://api.github.com/repos/softdevteam/grmtools/compare/{base}...{head}","merges_url":"https://api.github.com/repos/softdevteam/grmtools/merges","archive_url":"https://api.github.com/repos/softdevteam/grmtools/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/softdevteam/grmtools/downloads","issues_url":"https://api.github.com/repos/softdevteam/grmtools/issues{/number}","pulls_url":"https://api.github.com/repos/softdevteam/grmtools/pulls{/number}","milestones_url":"https://api.github.com/repos/softdevteam/grmtools/milestones{/number}","notifications_url":"https://api.github.com/repos/softdevteam/grmtools/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/softdevteam/grmtools/labels{/name}","releases_url":"https://api.github.com/repos/softdevteam/grmtools/releases{/id}","deployments_url":"https://api.github.com/repos/softdevteam/grmtools/deployments","created_at":"2018-08-06T14:50:43Z","updated_at":"2018-08-06T15:26:58Z","pushed_at":"2018-08-10T09:49:40Z","git_url":"git://github.com/softdevteam/grmtools.git","ssh_url":"git@github.com:softdevteam/grmtools.git","clone_url":"https://github.com/softdevteam/grmtools.git","svn_url":"https://github.com/softdevteam/grmtools","homepage":null,"size":821,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":1,"license":null,"forks":0,"open_issues":1,"watchers":0,"default_branch":"master"}},"base":{"label":"softdevteam:master","ref":"master","sha":"b715c835457bd78c9d9ea05d271525548ae6c2fc","user":{"login":"softdevteam","id":10271085,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwMjcxMDg1","avatar_url":"https://avatars0.githubusercontent.com/u/10271085?v=4","gravatar_id":"","url":"https://api.github.com/users/softdevteam","html_url":"https://github.com/softdevteam","followers_url":"https://api.github.com/users/softdevteam/followers","following_url":"https://api.github.com/users/softdevteam/following{/other_user}","gists_url":"https://api.github.com/users/softdevteam/gists{/gist_id}","starred_url":"https://api.github.com/users/softdevteam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/softdevteam/subscriptions","organizations_url":"https://api.github.com/users/softdevteam/orgs","repos_url":"https://api.github.com/users/softdevteam/repos","events_url":"https://api.github.com/users/softdevteam/events{/privacy}","received_events_url":"https://api.github.com/users/softdevteam/received_events","type":"Organization","site_admin":false},"repo":{"id":143743108,"node_id":"MDEwOlJlcG9zaXRvcnkxNDM3NDMxMDg=","name":"grmtools","full_name":"softdevteam/grmtools","owner":{"login":"softdevteam","id":10271085,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEwMjcxMDg1","avatar_url":"https://avatars0.githubusercontent.com/u/10271085?v=4","gravatar_id":"","url":"https://api.github.com/users/softdevteam","html_url":"https://github.com/softdevteam","followers_url":"https://api.github.com/users/softdevteam/followers","following_url":"https://api.github.com/users/softdevteam/following{/other_user}","gists_url":"https://api.github.com/users/softdevteam/gists{/gist_id}","starred_url":"https://api.github.com/users/softdevteam/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/softdevteam/subscriptions","organizations_url":"https://api.github.com/users/softdevteam/orgs","repos_url":"https://api.github.com/users/softdevteam/repos","events_url":"https://api.github.com/users/softdevteam/events{/privacy}","received_events_url":"https://api.github.com/users/softdevteam/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/softdevteam/grmtools","description":"Rust grammar tool libraries and binaries","fork":false,"url":"https://api.github.com/repos/softdevteam/grmtools","forks_url":"https://api.github.com/repos/softdevteam/grmtools/forks","keys_url":"https://api.github.com/repos/softdevteam/grmtools/keys{/key_id}","collaborators_url":"https://api.github.com/repos/softdevteam/grmtools/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/softdevteam/grmtools/teams","hooks_url":"https://api.github.com/repos/softdevteam/grmtools/hooks","issue_events_url":"https://api.github.com/repos/softdevteam/grmtools/issues/events{/number}","events_url":"https://api.github.com/repos/softdevteam/grmtools/events","assignees_url":"https://api.github.com/repos/softdevteam/grmtools/assignees{/user}","branches_url":"https://api.github.com/repos/softdevteam/grmtools/branches{/branch}","tags_url":"https://api.github.com/repos/softdevteam/grmtools/tags","blobs_url":"https://api.github.com/repos/softdevteam/grmtools/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/softdevteam/grmtools/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/softdevteam/grmtools/git/refs{/sha}","trees_url":"https://api.github.com/repos/softdevteam/grmtools/git/trees{/sha}","statuses_url":"https://api.github.com/repos/softdevteam/grmtools/statuses/{sha}","languages_url":"https://api.github.com/repos/softdevteam/grmtools/languages","stargazers_url":"https://api.github.com/repos/softdevteam/grmtools/stargazers","contributors_url":"https://api.github.com/repos/softdevteam/grmtools/contributors","subscribers_url":"https://api.github.com/repos/softdevteam/grmtools/subscribers","subscription_url":"https://api.github.com/repos/softdevteam/grmtools/subscription","commits_url":"https://api.github.com/repos/softdevteam/grmtools/commits{/sha}","git_commits_url":"https://api.github.com/repos/softdevteam/grmtools/git/commits{/sha}","comments_url":"https://api.github.com/repos/softdevteam/grmtools/comments{/number}","issue_comment_url":"https://api.github.com/repos/softdevteam/grmtools/issues/comments{/number}","contents_url":"https://api.github.com/repos/softdevteam/grmtools/contents/{+path}","compare_url":"https://api.github.com/repos/softdevteam/grmtools/compare/{base}...{head}","merges_url":"https://api.github.com/repos/softdevteam/grmtools/merges","archive_url":"https://api.github.com/repos/softdevteam/grmtools/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/softdevteam/grmtools/downloads","issues_url":"https://api.github.com/repos/softdevteam/grmtools/issues{/number}","pulls_url":"https://api.github.com/repos/softdevteam/grmtools/pulls{/number}","milestones_url":"https://api.github.com/repos/softdevteam/grmtools/milestones{/number}","notifications_url":"https://api.github.com/repos/softdevteam/grmtools/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/softdevteam/grmtools/labels{/name}","releases_url":"https://api.github.com/repos/softdevteam/grmtools/releases{/id}","deployments_url":"https://api.github.com/repos/softdevteam/grmtools/deployments","created_at":"2018-08-06T14:50:43Z","updated_at":"2018-08-06T15:26:58Z","pushed_at":"2018-08-10T09:49:40Z","git_url":"git://github.com/softdevteam/grmtools.git","ssh_url":"git@github.com:softdevteam/grmtools.git","clone_url":"https://github.com/softdevteam/grmtools.git","svn_url":"https://github.com/softdevteam/grmtools","homepage":null,"size":821,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":1,"license":null,"forks":0,"open_issues":1,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/softdevteam/grmtools/pulls/2"},"html":{"href":"https://github.com/softdevteam/grmtools/pull/2"},"issue":{"href":"https://api.github.com/repos/softdevteam/grmtools/issues/2"},"comments":{"href":"https://api.github.com/repos/softdevteam/grmtools/issues/2/comments"},"review_comments":{"href":"https://api.github.com/repos/softdevteam/grmtools/pulls/2/comments"},"review_comment":{"href":"https://api.github.com/repos/softdevteam/grmtools/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/softdevteam/grmtools/pulls/2/commits"},"statuses":{"href":"https://api.github.com/repos/softdevteam/grmtools/statuses/2a47105cb894c1abb656db8ac707a5f724d9e350"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"snim2"}}