-
-
Notifications
You must be signed in to change notification settings - Fork 366
return all referenced types for typeDefinition #2246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -5600,6 +5600,13 @@ fn addReferencedTypes( | |||
}, | |||
|
|||
.ip_index, .compile_error => {}, | |||
.either => {}, // TODO | |||
.either => |either| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels too easy to work...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe because it's just that simple. A test case would be need however.
This logic has since been moved here.
@@ -107,8 +107,8 @@ test "struct init" { | |||
|
|||
test "decl literal on generic type" { | |||
try testDefinition( | |||
\\fn Box(comptime T: type) type { | |||
\\ return <tdef>struct</tdef> { | |||
\\fn <tdef>Box</tdef>(comptime T: type) type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change desirable here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm mostly indifferent to this change. But if we are going to change this then we should at least remain consistent:
const <tdef>Foo</tdef> = struct {};
var <>foo = Foo{};
const Foo = <def>struct</def> {};
var foo = <>Foo{};
Either both go to Foo
or both go to struct
.
This will also need to handle cases like these where it needs to fall back to the old behavior:
const s = (struct { alpha: u32 }){ .alpha = 5 };
const Foo = (struct {});
var foo = Foo{};
try testDefinition( | ||
\\const <tdef>A</tdef> = struct {}; | ||
\\const <tdef>B</tdef> = struct {}; | ||
\\fn fo<>o(_: A) B {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fails, need to update the tests to work with array_of_DefinitionLink
This PR has gotten out of date so I am closing this for now. Feel free to rebase and open a new PR if there's still interest. |
this needs more (any) testing but just throwing it up for now, its quite possibly not too good! but i think the results can be pretty desirable.
closes #2052