Skip to content
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

Jump to bottom

Give error if using instance_sizeof on a generic type without type vars #5209

Merged
merged 1 commit into from Nov 3, 2017
Merged

Give error if using instance_sizeof on a generic type without type vars #5209

merged 1 commit into from Nov 3, 2017

Conversation

lbguilherme
Copy link
Contributor

Fixes #5207

@sdogruyol
Copy link
Member

LGTM 👍 ?

@RX14
Copy link
Contributor

RX14 commented Oct 30, 2017 •
edited

@sdogruyol in the interests of keeping noise down, can you not "LGTM" random prs, unless they need bumping (this PR isn't even a day old).

@sdogruyol
Copy link
Member

@RX14 they're mostly approved PRs not random..

@RX14
Copy link
Contributor

RX14 commented Oct 30, 2017

Ok but its still not helpful

@asterite
Copy link
Member

I agree, the LGTM end up in my mail box and when I think there's some new interesting reply or something relevant to the issue, it's just a 👍 . Let's avoid that.

asterite
asterite requested changes Oct 30, 2017
View reviewed changes
src/compiler/crystal/semantic/main_visitor.cr Outdated
raise "BUG: #{node} at #{node.location} should receive a type"
end

if type.is_a? GenericClassType
Copy link
Member

Choose a reason for hiding this comment

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

This should be GenericType. That's the base module for both generic classes and modules.

@lbguilherme
Copy link
Contributor Author

@asterite Done.

@RX14 RX14 added this to the Next milestone Nov 3, 2017
@RX14 RX14 merged commit 074d61c into crystal-lang:master Nov 3, 2017
ddrscott added a commit to ddrscott/crystal that referenced this pull request Apr 6, 2018
@ddrscott
add spec to reproduce sizeof(typeof(v)) bug
e1e8c6e 
After some `git bisect` work, it was discovered that the following bug
fix causes a regression on a patch which should be detectable.

> 074d61c is the first bad commit
>   commit 074d61c
>   Author: Guilherme Bernal <dev@lbguilherme.com>
>   Date:   Fri Nov 3 11:44:21 2017 -0300
>
>       give error if using instance_sizeof on a generic type without type vars (crystal-lang#5209)
>
>   :040000 040000 2fa5bdc54dead5c6c70f011e4e131f74bc170156 546f4b9d362c210b3b26f688e9314fb31ea64acb M	spec
>   :040000 040000 a3694241ab2bdfc038018e3280758fa3937b29de 80ec4ae5f0c337447208280893acb2fca45dbf5b M	src
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asterite asterite asterite approved these changes

@RX14 RX14 RX14 approved these changes

Assignees
No one assigned
Labels
kind:bug topic:compiler
Projects
None yet
Milestone
0.24.1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lbguilherme @sdogruyol @RX14 @asterite