{"action":"created","issue":{"url":"https://api.github.com/repos/ucb-bar/chisel3/issues/186","repository_url":"https://api.github.com/repos/ucb-bar/chisel3","labels_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/186/labels{/name}","comments_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/186/comments","events_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/186/events","html_url":"https://github.com/ucb-bar/chisel3/pull/186","id":154369172,"number":186,"title":"Source locators","user":{"login":"ducky64","id":1069863,"avatar_url":"https://avatars.githubusercontent.com/u/1069863?v=3","gravatar_id":"","url":"https://api.github.com/users/ducky64","html_url":"https://github.com/ducky64","followers_url":"https://api.github.com/users/ducky64/followers","following_url":"https://api.github.com/users/ducky64/following{/other_user}","gists_url":"https://api.github.com/users/ducky64/gists{/gist_id}","starred_url":"https://api.github.com/users/ducky64/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ducky64/subscriptions","organizations_url":"https://api.github.com/users/ducky64/orgs","repos_url":"https://api.github.com/users/ducky64/repos","events_url":"https://api.github.com/users/ducky64/events{/privacy}","received_events_url":"https://api.github.com/users/ducky64/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ucb-bar/chisel3/labels/enhancement","name":"enhancement","color":"84b6eb"}],"state":"open","locked":false,"assignee":{"login":"sdtwigg","id":1015800,"avatar_url":"https://avatars.githubusercontent.com/u/1015800?v=3","gravatar_id":"","url":"https://api.github.com/users/sdtwigg","html_url":"https://github.com/sdtwigg","followers_url":"https://api.github.com/users/sdtwigg/followers","following_url":"https://api.github.com/users/sdtwigg/following{/other_user}","gists_url":"https://api.github.com/users/sdtwigg/gists{/gist_id}","starred_url":"https://api.github.com/users/sdtwigg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdtwigg/subscriptions","organizations_url":"https://api.github.com/users/sdtwigg/orgs","repos_url":"https://api.github.com/users/sdtwigg/repos","events_url":"https://api.github.com/users/sdtwigg/events{/privacy}","received_events_url":"https://api.github.com/users/sdtwigg/received_events","type":"User","site_admin":false},"milestone":null,"comments":37,"created_at":"2016-05-12T00:01:15Z","updated_at":"2016-05-20T21:07:00Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ucb-bar/chisel3/pulls/186","html_url":"https://github.com/ucb-bar/chisel3/pull/186","diff_url":"https://github.com/ucb-bar/chisel3/pull/186.diff","patch_url":"https://github.com/ucb-bar/chisel3/pull/186.patch"},"body":"This basically adds the SourceInfo trait and several concrete classes for when source information isn't available and a concrete class for source information from a Scala source line.\r\n\r\nSource information is also added to the FIRRTL IR nodes and emitter as an explicit parameter. Source information is propagated through the Chisel frontend and internals as an implicit parameter. For methods that don't return Unit (aka nothing), a macro transform explicitly adds the SourceInfo parameter as implicitly[SourceInfo] so apply() calls can be chained after. The transformed versions of methods are of the same name, except with `do_` prepended. One of the macros (`SourceInfoTransform`) automatically detects the function name, which eliminates a lot of boilerplate.\r\n\r\nThe package split is up for debate. There is two packages:\r\ncoreMacros defines the transforms (that add explicit implicits: `implicitly[SourceInfo]`) and nothing else. This allows the macros to be used and invoked at the next level, which prevents a lot of ugly `do_*` calls everywhere. Turns out you just need the macro transform definitions, not the functions that are defined as macros, in a different compilation unit.\r\nchiselFrontend defines the core data types. ~~There's no real reason this needs to be separate from the rest of Chisel (other than the assert macros), but I think there's a certain elegance for splitting core data types from helpers (Driver and friends) and utils.~~ chiselFrontend is split out from the rest of utils as a layer of safety, since the `SourceInfo.materialize` macro is defined in chiselFrontend, this prevents the system from generating source locators in Chisel core, which is almost certainly a bug.\r\n\r\nMems a work in progress, and is dependent on #189. Will be future work, probably near future work.\r\n\r\nDefining how library writers would have source locators for entry points into their libraries is future work.\r\n\r\n~~Only tested with Chisel unit tests. Someone should test with a more complex design, like rocket chip. **Requires a FIRRTL compiler change to support the source info annotations. This will not yet work.** However, you can test correctness without source locators passed through to FIRRTL by eliminating the part in Emitter.scala where the locators are actually generated.~~\r\n\r\nucb-bar/firrtl#166 is in, so this should work now!\r\n\r\nImplementation for #147 \r\nPartially based on implementation sketch in #158"},"comment":{"url":"https://api.github.com/repos/ucb-bar/chisel3/issues/comments/220718674","html_url":"https://github.com/ucb-bar/chisel3/pull/186#issuecomment-220718674","issue_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/186","id":220718674,"user":{"login":"sdtwigg","id":1015800,"avatar_url":"https://avatars.githubusercontent.com/u/1015800?v=3","gravatar_id":"","url":"https://api.github.com/users/sdtwigg","html_url":"https://github.com/sdtwigg","followers_url":"https://api.github.com/users/sdtwigg/followers","following_url":"https://api.github.com/users/sdtwigg/following{/other_user}","gists_url":"https://api.github.com/users/sdtwigg/gists{/gist_id}","starred_url":"https://api.github.com/users/sdtwigg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdtwigg/subscriptions","organizations_url":"https://api.github.com/users/sdtwigg/orgs","repos_url":"https://api.github.com/users/sdtwigg/repos","events_url":"https://api.github.com/users/sdtwigg/events{/privacy}","received_events_url":"https://api.github.com/users/sdtwigg/received_events","type":"User","site_admin":false},"created_at":"2016-05-20T21:07:00Z","updated_at":"2016-05-20T21:07:00Z","body":"My preference is 'option 4':\r\nAssert doesn't take an implicit SourceInfo and both its message information and SourceInfo is filled in by the macroApplication details from it 'current' macro function.\r\n\r\nI don't believe the scenario where the chisel developer debugging an issue with assert BOTH understands the 'current' assert transform macro to build up the message AND how implicits and the source info implicit materializer works BUT can't fathom how a macro that does both at once works. Assert is already going to be confusing because regardless of what you choose you are layering another macro application on top of it. The fact that you do this to add source information to the message string is already making it non-uniform to other functions. \r\n\r\nRegardless, my main hangup was that you had macros overriding macros and that has been resolved. I don't care enough about the dual construction of SourceInfo in assert to argue this further."}}
{"actor":{"display_login":"sdtwigg"}}