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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow multiple constants assignment #3507

Closed
wants to merge 1 commit into from
Closed

Disallow multiple constants assignment #3507

wants to merge 1 commit into from

Conversation

sdogruyol
Copy link
Member

@sdogruyol sdogruyol commented Nov 6, 2016

This is my first time deep diving into the compiler 馃槃 Fixes #3466

I added a spec to spec/compiler/semantic/const_spec.cr with assert_error, it's working but failing.

Failures:

  1) Semantic: const errors on multiple constant assignment (#3466)

       Syntax error in :2: Multiple assignment is not allowed for constants
       [4386931682] *CallStack::unwind:Array(Pointer(Void)) +82
       [4386931585] *CallStack#initialize:Array(Pointer(Void)) +17
       [4386931544] *CallStack::new:CallStack +40
       [4386881281] *raise<Crystal::SyntaxException>:NoReturn +17
       [4395110195] *Crystal::Parser@Crystal::Lexer#raise<String, Int32, Int32, (Crystal::VirtualFile | String | Nil)>:NoReturn +35
       [4395110158] *Crystal::Parser@Crystal::Lexer#raise<String>:NoReturn +62
       [4395126494] *Crystal::Parser#parse_multi_assign:Crystal::ASTNode+ +206
       [4395125219] *Crystal::Parser#parse_expressions_internal:Crystal::ASTNode+ +163
       [4395125012] *Crystal::Parser#parse_expressions:Crystal::ASTNode+ +52
       [4395075326] *Crystal::Parser#parse:Crystal::ASTNode+ +30
       [4386881258] *parse<String, Bool>:Crystal::ASTNode+ +58
       [4386881183] *parse<String>:Crystal::ASTNode+ +31
       [4386904867] *assert_error<String, String, Bool>:Nil +83
       [4386904775] *assert_error<String, String>:Nil +39
       [4386909101] ~procProc(Nil)@./spec/compiler/semantic/const_spec.cr:241 +29
       [4386879831] *it<String, String, Int32, &Proc(Nil)>:(Array(Spec::Result) | Nil) +375
       [4386879432] ~procProc(Nil)@./spec/compiler/semantic/const_spec.cr:3 +2088
       [4387385321] *Spec::RootContext::describe<String, String, Int32, &Proc(Nil)>:Spec::Context+ +361
       [4386877289] *describe<String, String, Int32, &Proc(Nil)>:Spec::Context+ +9
       [4386784117] __crystal_main +4661
       [4386855336] main +40

Finished in 1.41 seconds
28 examples, 0 failures, 1 errors, 0 pending

Any help wecome 馃憤

@sdogruyol sdogruyol changed the title Disallow multiple assignment for constants Disallow multiple constant assignments Nov 6, 2016
@sdogruyol sdogruyol changed the title Disallow multiple constant assignments Disallow multiple constants assignment Nov 6, 2016
@@ -237,4 +237,8 @@ describe "Semantic: const" do
FOO
)) { types["Foo"] }
end

it "errors on multiple constant assignment (#3466)" do
Copy link
Member

Choose a reason for hiding this comment

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

This spec isn't needed: the parser already gives an error for this, which happens before the semantic pass

@asterite
Copy link
Member

asterite commented Dec 7, 2016

@sdogruyol I had a bit of time to review this (it was low prio) and your changes look really good! I made a comment, after you fix that I'll merge it :-)

@sdogruyol
Copy link
Member Author

Closing this for #3728

@sdogruyol sdogruyol closed this Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants