-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Bug: initialize with {% @type %} #6077
Bug: initialize with {% @type %} #6077
Conversation
@@ -4429,6 +4429,21 @@ describe "Semantic: instance var" do | |||
), inject_primitives: false) { types["Foo"] } | |||
end | |||
|
|||
it "is more permissive with macro def initialize, bug with named args" do |
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.
Reference this PR number in the spec name?
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.
That's a bit recursive :-)
I usually reference the issue number (bug), but there's none here (well, there's that comment...). In any case, the blame of that line will lead to this commit, and in GitHub it will relate to this PR.
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.
git blame can easilly be thrown off by a few refactors or file renames, far down the line.
@@ -108,7 +108,7 @@ struct Crystal::TypeDeclarationProcessor | |||
# removed if an explicit type is found (in remove_error). | |||
@errors = {} of Type => Hash(String, Error) | |||
|
|||
# Types that have a single macro def initialize | |||
# Types whose initialize methods all are macro defs |
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.
nit: s/all are/are all/
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.
Thanks, fixed.
Fixes a bug found in #6063 (comment)
The compiler has a secret rule that's not documented (but probably should): if an
initialize
uses@type
in macros, then checking what instance variables are initialized is delayed until the semantic phase, only for those methods. This allows the definition ofinitialize
methods that are based on a type's instance vars, for example.But there were a few bugs with that. This PR fixes those.