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

Pending leftovers to support // and &-ops in multiple places #7628

Merged
merged 4 commits into from
Apr 10, 2019

Conversation

bcardiff
Copy link
Member

@bcardiff bcardiff commented Apr 4, 2019

Some missing bits to fully support // in all places were / is supported today.

Ref: #2968

@bcardiff bcardiff force-pushed the feature/macro-int-div branch from 0c54cb8 to e540560 Compare April 5, 2019 13:51
@bcardiff
Copy link
Member Author

bcardiff commented Apr 5, 2019

Thanks for all the reviews, but the CI wasn't happy.
There are other left overs I've just found regarding the &-ops and //. I will send them in another PR.

@bcardiff bcardiff changed the title Add // to macros and math interpreter for resolving constants Pending leftovers to support // and &-ops in multiple places Apr 5, 2019
@@ -83,11 +83,20 @@ describe "Semantic: static array" do

it "types static array new with size being a computed constant" do
assert_type(%(
struct Int
def //(other : Int)
Copy link
Member Author

Choose a reason for hiding this comment

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

I've found it a bit odd that the value of SIZE can be computed by the math interpreter but the definition is still needed. It's similar to what is done in https://github.com/crystal-lang/crystal/blob/master/spec/compiler/semantic/const_spec.cr#L237-L239 .

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just needed for the typing phase, so the implementation can be just a 0. I've just pushed that change

@bcardiff bcardiff force-pushed the feature/macro-int-div branch from 71be379 to 5397e39 Compare April 5, 2019 16:38
@bcardiff bcardiff requested a review from asterite April 5, 2019 16:39
@bcardiff bcardiff merged commit 23dd64e into crystal-lang:master Apr 10, 2019
@bcardiff bcardiff deleted the feature/macro-int-div branch April 11, 2019 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants