{"action":"created","issue":{"url":"https://api.github.com/repos/freechipsproject/chisel3/issues/946","repository_url":"https://api.github.com/repos/freechipsproject/chisel3","labels_url":"https://api.github.com/repos/freechipsproject/chisel3/issues/946/labels{/name}","comments_url":"https://api.github.com/repos/freechipsproject/chisel3/issues/946/comments","events_url":"https://api.github.com/repos/freechipsproject/chisel3/issues/946/events","html_url":"https://github.com/freechipsproject/chisel3/pull/946","id":382468046,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMyMTY5NTUw","number":946,"title":"Ignore empty aggregates elements when binding aggregate direction","user":{"login":"jackkoenig","id":8584418,"node_id":"MDQ6VXNlcjg1ODQ0MTg=","avatar_url":"https://avatars3.githubusercontent.com/u/8584418?v=4","gravatar_id":"","url":"https://api.github.com/users/jackkoenig","html_url":"https://github.com/jackkoenig","followers_url":"https://api.github.com/users/jackkoenig/followers","following_url":"https://api.github.com/users/jackkoenig/following{/other_user}","gists_url":"https://api.github.com/users/jackkoenig/gists{/gist_id}","starred_url":"https://api.github.com/users/jackkoenig/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jackkoenig/subscriptions","organizations_url":"https://api.github.com/users/jackkoenig/orgs","repos_url":"https://api.github.com/users/jackkoenig/repos","events_url":"https://api.github.com/users/jackkoenig/events{/privacy}","received_events_url":"https://api.github.com/users/jackkoenig/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-20T02:05:27Z","updated_at":"2018-11-20T17:53:23Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/freechipsproject/chisel3/pulls/946","html_url":"https://github.com/freechipsproject/chisel3/pull/946","diff_url":"https://github.com/freechipsproject/chisel3/pull/946.diff","patch_url":"https://github.com/freechipsproject/chisel3/pull/946.patch"},"body":"Previously, including an empty aggregate in a Bundle would cause\r\na MixedDirectionAggregateException because it has no elements and thus\r\ndoesn't have a direction\r\n\r\n<!--\r\nPlease select the item best describing the pull request in each category and delete the other items.\r\n-->\r\n**Related issue**: <!-- if applicable -->\r\n\r\n<!-- choose one -->\r\n**Type of change**: bug report and fix\r\n\r\n<!-- choose one -->\r\n**Impact**: API addition? Stuff that doesn't work (but should) now does\r\n\r\n<!-- choose one -->\r\n**Development Phase**: implementation\r\n\r\n**Release Notes**\r\nEmpty aggregates no longer cause MixedDirectionAggregateException when included as members of Bundles\r\n\r\nh/t @davidbiancolin for identifying and reducing the problem\r\n"},"comment":{"url":"https://api.github.com/repos/freechipsproject/chisel3/issues/comments/440370427","html_url":"https://github.com/freechipsproject/chisel3/pull/946#issuecomment-440370427","issue_url":"https://api.github.com/repos/freechipsproject/chisel3/issues/946","id":440370427,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0MDM3MDQyNw==","user":{"login":"jackkoenig","id":8584418,"node_id":"MDQ6VXNlcjg1ODQ0MTg=","avatar_url":"https://avatars3.githubusercontent.com/u/8584418?v=4","gravatar_id":"","url":"https://api.github.com/users/jackkoenig","html_url":"https://github.com/jackkoenig","followers_url":"https://api.github.com/users/jackkoenig/followers","following_url":"https://api.github.com/users/jackkoenig/following{/other_user}","gists_url":"https://api.github.com/users/jackkoenig/gists{/gist_id}","starred_url":"https://api.github.com/users/jackkoenig/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jackkoenig/subscriptions","organizations_url":"https://api.github.com/users/jackkoenig/orgs","repos_url":"https://api.github.com/users/jackkoenig/repos","events_url":"https://api.github.com/users/jackkoenig/events{/privacy}","received_events_url":"https://api.github.com/users/jackkoenig/received_events","type":"User","site_admin":false},"created_at":"2018-11-20T17:53:23Z","updated_at":"2018-11-20T17:53:23Z","author_association":"CONTRIBUTOR","body":"I grant that the given empty Bundle test case looks a little bit funky, but I'm not really sure how you distinguish it from more reasonable looking yet functionally equivalent cases:\r\n```scala\r\nval io = IO(new Bundle {\r\n  val in = Input(UInt(8.W))\r\n  val a = new Bundle { }\r\n  val b = new Bundle {\r\n    val foo = if (false) Some(Input(UInt(8.W))) else None\r\n  }\r\n  val c = MyCustomRecord(List.fill(0)(Output(UInt(8.W))))\r\n})\r\n```\r\nWhile I see the argument for making `val a` above illegal, I don't think it's possible to do so without making `b` and `c` illegal which would be very problematic. We need a test case for empty Records which is what I was using `new Bundle { }` to accomplish.\r\n\r\nI agree that Vec directionality logic should run on the sample element if only to save unnecessary computation. I can change that as part of this PR or it could be a follow on?"}}
{"actor":{"display_login":"jackkoenig"}}