Skip to content

Fix type def reopening type from parent namespace#11208

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
straight-shoota:fix/type-def-reopen-from-parent-namespace
Apr 13, 2024
Merged

Fix type def reopening type from parent namespace#11208
straight-shoota merged 2 commits intocrystal-lang:masterfrom
straight-shoota:fix/type-def-reopen-from-parent-namespace

Conversation

@straight-shoota
Copy link
Copy Markdown
Member

Resolves #11181

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Sep 14, 2021
") { char }
end

it "type def does not reopen type from parent namespace (#11181)" do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 "type def" is confusing because typedef is a thing in the language. Maybe "type declaration" or just "class def" or "class declaration"?

Copy link
Copy Markdown
Member Author

@straight-shoota straight-shoota Sep 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took that term from the lookup_type_def method. But yeah, I also thought it might be considered ambiguous.

A type declaration is var : Type, so I don't think that's a better solution. class works here, and I suppose we can use module in the other spec (I didn't add specs for all kinds of types, but I think that's fine b/c the code is identical).

But it would be better to have a generic term for definitions of all kinds of types (class, struct, module, ...). And I think "type definition" is a natural solution for that with "type" being the abstraction for all kinds of types and it being a "definition".

If we go along with #10031 the term would be free from overloads =) For now, I'd propose to leave it as is. In this context, the meaning is unambiguous.

@straight-shoota straight-shoota modified the milestones: 1.12.0, 1.13.0 Apr 2, 2024
@straight-shoota
Copy link
Copy Markdown
Member Author

straight-shoota commented Apr 4, 2024

This is a bug fix which we could still merge for 1.12.0, but there's a non-dismissible chance for unforseen effects with such a change in the type grammar. This patch has been sitting for years, so there seems to be no rush to release it.

Copy link
Copy Markdown
Collaborator

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge right after 1.12 is released, so we can have some time in nightly before releasing 1.13 and we don't push it forever.

@straight-shoota straight-shoota merged commit 0efbf53 into crystal-lang:master Apr 13, 2024
@straight-shoota straight-shoota deleted the fix/type-def-reopen-from-parent-namespace branch April 13, 2024 10:01
cyangle added a commit to cyangle/jennifer.cr that referenced this pull request Aug 8, 2024
cyangle added a commit to cyangle/jennifer.cr that referenced this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Namespaced type with nested name reopens type in parent namespace

4 participants