Skip to content

Add &+ &- &* &** operators parsing #6329

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.

Sign up for GitHub

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

Merged
bcardiff merged 6 commits into from
Jul 9, 2018
Merged

Add &+ &- &* &** operators parsing #6329

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
71f72f4
Add &+ &- &* &** operators parsing
bcardiff Jun 25, 2018
12e0e54
Update test description
bcardiff Jul 4, 2018
25cab7c
Add lookahead to parse &->expr as &(->expr)
bcardiff Jul 4, 2018
ad0a3c8
Allow &+ as prefix operator
bcardiff Jul 4, 2018
96db8fd
Add formatter specs and rules
bcardiff Jul 4, 2018
d3b1c0d
Rollback changes in semantic/block_spec.cr
bcardiff Jul 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions spec/compiler/codegen/proc_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,13 +600,13 @@ describe "Code gen: proc" do
)).to_i.should eq(1)
end

it "passes proc as &-> to method that yields" do
it "passes proc as &(->expr) to method that yields" do

@straight-shoota straight-shoota Jul 4, 2018

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.

Why not add a lookahead to avoid this breaking change?

@bcardiff bcardiff Jul 4, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because the current lexer does not do lookahead AFAIK. So is changing a bit the nature of it for this case. And that syntax is not wide used.

@straight-shoota straight-shoota Jul 4, 2018

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.

peek_next_char?

@asterite asterite Jul 4, 2018 •
edited
Loading

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.

You can use this diff:

diff --git a/spec/compiler/codegen/proc_spec.cr b/spec/compiler/codegen/proc_spec.cr
index bee6aa1f0..4e08c075c 100644
--- a/spec/compiler/codegen/proc_spec.cr
+++ b/spec/compiler/codegen/proc_spec.cr
@@ -600,13 +600,13 @@ describe "Code gen: proc" do
       )).to_i.should eq(1)
   end
 
-  it "passes proc as &(->expr) to method that yields" do
+  it "passes proc as &->expr to method that yields" do
     run(%(
       def foo
         yield
       end
 
-      foo &(->{ 123 })
+      foo &->{ 123 }
       )).to_i.should eq(123)
   end
 
diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr
index 3493e4c83..168b18344 100644
--- a/spec/compiler/parser/parser_spec.cr
+++ b/spec/compiler/parser/parser_spec.cr
@@ -1085,6 +1085,8 @@ module Crystal
     it_parses "->foo=", ProcPointer.new(nil, "foo=")
     it_parses "foo = 1; ->foo.foo=", [Assign.new("foo".var, 1.int32), ProcPointer.new("foo".var, "foo=")]
 
+    it_parses "foo &->bar", Call.new(nil, "foo", block_arg: ProcPointer.new(nil, "bar"))
+
     it_parses "foo.bar = {} of Int32 => Int32", Call.new("foo".call, "bar=", HashLiteral.new(of: HashLiteral::Entry.new("Int32".path, "Int32".path)))
 
     it_parses "alias Foo = Bar", Alias.new("Foo", "Bar".path)
diff --git a/src/compiler/crystal/syntax/lexer.cr b/src/compiler/crystal/syntax/lexer.cr
index c02bc8364..915ab8da7 100644
--- a/src/compiler/crystal/syntax/lexer.cr
+++ b/src/compiler/crystal/syntax/lexer.cr
@@ -596,11 +596,18 @@ module Crystal
             @token.type = :"&+"
           end
         when '-'
-          case next_char
-          when '='
-            next_char :"&-="
+          # Check if '>' comes after '&-', making it '&->'.
+          # We want to parse that like '&(->...)',
+          # so we only return '&' for now.
+          if peek_next_char == '>'
+            @token.type = :"&"
           else
-            @token.type = :"&-"
+            case next_char
+            when '='
+              next_char :"&-="
+            else
+              @token.type = :"&-"
+            end
           end
         when '*'
           case next_char

@asterite asterite Jul 4, 2018

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.

Update: I just edited the diff because you can call peek_next_char instead of reader.peek_next_char

@asterite asterite Jul 4, 2018

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.

Updated again to revert the change in the spec that uses &-> where parentheses were added.

@bcardiff bcardiff Jul 4, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Diff merged. Thanks @asterite

run(%(
def foo
yield
end

foo &->{ 123 }
foo &(->{ 123 })
)).to_i.should eq(123)
end

Expand Down
7 changes: 5 additions & 2 deletions spec/compiler/lexer/lexer_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,18 +249,21 @@ describe "Lexer" do
:"|", :"{", :"}", :"?", :":", :"+=", :"-=", :"*=", :"%=", :"&=",
:"|=", :"^=", :"**=", :"<<", :">>", :"%", :"&", :"|", :"^", :"**", :"<<=",
:">>=", :"~", :"[]", :"[]=", :"[", :"]", :"::", :"<=>", :"=>", :"||=",
:"&&=", :"===", :";", :"->", :"[]?", :"{%", :"{{", :"%}", :"@[", :"!~"]
:"&&=", :"===", :";", :"->", :"[]?", :"{%", :"{{", :"%}", :"@[", :"!~",
:"&+", :"&-", :"&*", :"&**", :"&+=", :"&-=", :"&*="]
it_lexes "!@foo", :"!"
it_lexes "+@foo", :"+"
it_lexes "-@foo", :"-"
it_lexes "&-@foo", :"&-"
it_lexes_const "Foo"
it_lexes_instance_var "@foo"
it_lexes_class_var "@@foo"
it_lexes_globals ["$foo", "$FOO", "$_foo", "$foo123"]
it_lexes_symbols [":foo", ":foo!", ":foo?", ":foo=", ":\"foo\"", ":かたな", ":+", ":-", ":*", ":/",
":==", ":<", ":<=", ":>", ":>=", ":!", ":!=", ":=~", ":!~", ":&", ":|",
":^", ":~", ":**", ":>>", ":<<", ":%", ":[]", ":[]?", ":[]=", ":<=>", ":===",
]
":&+", ":&-", ":&*", ":&**"]

it_lexes_global_match_data_index ["$1", "$10", "$1?", "$23?"]

it_lexes "$~", :"$~"
Expand Down
6 changes: 4 additions & 2 deletions spec/compiler/normalize/op_assign_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
require "../../spec_helper"

describe "Normalize: op assign" do
it "normalizes var +=" do
assert_normalize "a = 1; a += 2", "a = 1\na = a + 2"
["+", "-", "*", "&+", "&-", "&*"].each do |op|

@jkthorne jkthorne Jul 5, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

seem like in the rest of this file each test case is broken out. I think it might make it easier to understand if they are different test cases.

it "normalizes var #{op}=" do
assert_normalize "a = 1; a #{op}= 2", "a = 1\na = a #{op} 2"
end
end

it "normalizes var ||=" do
Expand Down
11 changes: 7 additions & 4 deletions spec/compiler/parser/parser_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ module Crystal
it_parses "~ 1", Call.new(1.int32, "~")
it_parses "1 && 2", And.new(1.int32, 2.int32)
it_parses "1 || 2", Or.new(1.int32, 2.int32)
it_parses "&- 1", Call.new(1.int32, "&-")

@Sija Sija Jul 4, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is support for &+ 1 left out intentionally?

@bcardiff bcardiff Jul 4, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, I just added &- because there is a prefix operation -. I will add the specs.

@Sija Sija Jul 4, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I asked since AFAIR there's also prefix + operator.

@asterite asterite Jul 4, 2018

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.

Can prefix - or + overflow? If not, maybe it's not worth adding these.

@yxhuvud yxhuvud Jul 4, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@asterite - can on 2 complement as 0 is taken from the positive side. Dunno about systems with other bases.

@bcardiff bcardiff Jul 4, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note that I am not planning on implementing prefix &-, but since unitary - is allowed, &- should also work. It is only used in an stdlib string operation that can be replaced by 0 &- expr


it_parses "1 <=> 2", Call.new(1.int32, "<=>", 2.int32)
it_parses "1 !~ 2", Call.new(1.int32, "!~", 2.int32)
Expand Down Expand Up @@ -419,7 +420,7 @@ module Crystal
it_parses "f.x = Foo.new", Call.new("f".call, "x=", [Call.new("Foo".path, "new")] of ASTNode)
it_parses "f.x = - 1", Call.new("f".call, "x=", [Call.new(1.int32, "-")] of ASTNode)

["+", "-", "*", "/", "%", "|", "&", "^", "**", "<<", ">>"].each do |op|
["+", "-", "*", "/", "%", "|", "&", "^", "**", "<<", ">>", "&+", "&-", "&*"].each do |op|
it_parses "f.x #{op}= 2", OpAssign.new(Call.new("f".call, "x"), op, 2.int32)
end

Expand All @@ -430,9 +431,10 @@ module Crystal
it_parses "def %(); end;", Def.new("%")
it_parses "def /(); end;", Def.new("/")

["<<", "<", "<=", "==", ">>", ">", ">=", "+", "-", "*", "/", "%", "|", "&", "^", "**", "===", "=~", "!~"].each do |op|
["<<", "<", "<=", "==", ">>", ">", ">=", "+", "-", "*", "/", "%", "|", "&", "^", "**", "===", "=~", "!~", "&+", "&-", "&*", "&**"].each do |op|
it_parses "1 #{op} 2", Call.new(1.int32, op, 2.int32)
it_parses "n #{op} 2", Call.new("n".call, op, 2.int32)
it_parses "def #{op}(); end", Def.new(op)
end

["bar", "+", "-", "*", "/", "<", "<=", "==", ">", ">=", "%", "|", "&", "^", "**", "===", "=~", "!~"].each do |name|
Expand All @@ -441,7 +443,7 @@ module Crystal
it_parses "foo.#{name}(1, 2)", Call.new("foo".call, name, 1.int32, 2.int32)
end

["+", "-", "*", "/", "%", "|", "&", "^", "**", "<<", ">>"].each do |op|
["+", "-", "*", "/", "%", "|", "&", "^", "**", "<<", ">>", "&+", "&-", "&*"].each do |op|
it_parses "a = 1; a #{op}= 1", [Assign.new("a".var, 1.int32), OpAssign.new("a".var, op, 1.int32)]
it_parses "a = 1; a #{op}=\n1", [Assign.new("a".var, 1.int32), OpAssign.new("a".var, op, 1.int32)]
it_parses "a.b #{op}=\n1", OpAssign.new(Call.new("a".call, "b"), op, 1.int32)
Expand Down Expand Up @@ -642,7 +644,8 @@ module Crystal
assert_syntax_error "#{keyword} ? 1 : 2", "void value expression"
assert_syntax_error "+#{keyword}", "void value expression"

["<<", "<", "<=", "==", ">>", ">", ">=", "+", "-", "*", "/", "%", "|", "&", "^", "**", "==="].each do |op|
["<<", "<", "<=", "==", ">>", ">", ">=", "+", "-", "*", "/", "%", "|",
"&", "^", "**", "===", "&+", "&-", "&*", "&**"].each do |op|
assert_syntax_error "#{keyword} #{op} 1", "void value expression"
end

Expand Down
4 changes: 2 additions & 2 deletions spec/compiler/semantic/block_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ describe "Block inference" do
)) { types["Moo"].types["Bar"] }
end

it "passes &->f" do
it "passes &(->f)" do

@straight-shoota straight-shoota Jul 5, 2018

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.

These changes should no longer be necessary.

assert_type(%(
def foo
end
Expand All @@ -924,7 +924,7 @@ describe "Block inference" do
1
end

bar &->foo
bar &(->foo)
)) { int32 }
end

Expand Down
39 changes: 38 additions & 1 deletion src/compiler/crystal/syntax/lexer.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,21 @@ module Crystal
symbol ">"
end
when '&'
next_char_and_symbol "&"
case next_char
when '+'
next_char_and_symbol "&+"
when '-'
next_char_and_symbol "&-"
when '*'
case next_char
when '*'
next_char_and_symbol "&**"
else
symbol "&*"
end
else
symbol "&"
end
when '|'
next_char_and_symbol "|"
when '^'
Expand Down Expand Up @@ -574,6 +588,29 @@ module Crystal
end
when '='
next_char :"&="
when '+'
case next_char
when '='
next_char :"&+="
else
@token.type = :"&+"
end
when '-'
case next_char
when '='
next_char :"&-="
else
@token.type = :"&-"
end
when '*'
case next_char
when '*'
next_char :"&**"
when '='
next_char :"&*="
else
@token.type = :"&*"
end
else
@token.type = :"&"
end
Expand Down
18 changes: 9 additions & 9 deletions src/compiler/crystal/syntax/parser.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ module Crystal
atomic.doc = doc
atomic
end
when :"+=", :"-=", :"*=", :"/=", :"%=", :"|=", :"&=", :"^=", :"**=", :"<<=", :">>=", :"||=", :"&&="
when :"+=", :"-=", :"*=", :"/=", :"%=", :"|=", :"&=", :"^=", :"**=", :"<<=", :">>=", :"||=", :"&&=", :"&+=", :"&-=", :"&*="
unexpected_token unless allow_ops

break unless can_be_assigned?(atomic)
Expand Down Expand Up @@ -502,7 +502,7 @@ module Crystal
case @token.type
when :SPACE
next_token
when :"+", :"-"
when :"+", :"-", :"&+", :"&-"
check_void_value left, location

method = @token.type.to_s
Expand Down Expand Up @@ -531,13 +531,13 @@ module Crystal
end
end

parse_operator :mul_or_div, :pow, "Call.new left, method, [right] of ASTNode, name_column_number: method_column_number", ":\"*\", :\"/\", :\"%\""
parse_operator :pow, :prefix, "Call.new left, method, [right] of ASTNode, name_column_number: method_column_number", ":\"**\""
parse_operator :mul_or_div, :pow, "Call.new left, method, [right] of ASTNode, name_column_number: method_column_number", %(:"*", :"/", :"%", :"&*")
parse_operator :pow, :prefix, "Call.new left, method, [right] of ASTNode, name_column_number: method_column_number", %(:"**", :"&**")

def parse_prefix
column_number = @token.column_number
case token_type = @token.type
when :"!", :"+", :"-", :"~"
when :"!", :"+", :"-", :"~", :"&-"
location = @token.location
next_token_skip_space_or_newline
check_void_expression_keyword
Expand All @@ -552,7 +552,7 @@ module Crystal
end
end

AtomicWithMethodCheck = [:IDENT, :CONST, :"+", :"-", :"*", :"/", :"%", :"|", :"&", :"^", :"**", :"<<", :"<", :"<=", :"==", :"!=", :"=~", :"!~", :">>", :">", :">=", :"<=>", :"===", :"[]", :"[]=", :"[]?", :"["]
AtomicWithMethodCheck = [:IDENT, :CONST, :"+", :"-", :"*", :"/", :"%", :"|", :"&", :"^", :"**", :"<<", :"<", :"<=", :"==", :"!=", :"=~", :"!~", :">>", :">", :">=", :"<=>", :"===", :"[]", :"[]=", :"[]?", :"[", :"&+", :"&-", :"&*", :"&**"]

def parse_atomic_with_method
location = @token.location
Expand Down Expand Up @@ -666,7 +666,7 @@ module Crystal

atomic = Call.new(atomic, "#{name}=", [arg] of ASTNode, name_column_number: name_column_number).at(location)
next
when :"+=", :"-=", :"*=", :"/=", :"%=", :"|=", :"&=", :"^=", :"**=", :"<<=", :">>=", :"||=", :"&&="
when :"+=", :"-=", :"*=", :"/=", :"%=", :"|=", :"&=", :"^=", :"**=", :"<<=", :">>=", :"||=", :"&&=", :"&+=", :"&-=", :"&*="
method = @token.type.to_s.byte_slice(0, @token.type.to_s.size - 1)
next_token_skip_space_or_newline
value = parse_op_assign
Expand Down Expand Up @@ -3197,8 +3197,8 @@ module Crystal
exp
end

DefOrMacroCheck1 = [:IDENT, :CONST, :"<<", :"<", :"<=", :"==", :"===", :"!=", :"=~", :"!~", :">>", :">", :">=", :"+", :"-", :"*", :"/", :"!", :"~", :"%", :"&", :"|", :"^", :"**", :"[]", :"[]=", :"<=>", :"[]?"]
DefOrMacroCheck2 = [:"<<", :"<", :"<=", :"==", :"===", :"!=", :"=~", :"!~", :">>", :">", :">=", :"+", :"-", :"*", :"/", :"!", :"~", :"%", :"&", :"|", :"^", :"**", :"[]", :"[]?", :"[]=", :"<=>"]
DefOrMacroCheck1 = [:IDENT, :CONST, :"<<", :"<", :"<=", :"==", :"===", :"!=", :"=~", :"!~", :">>", :">", :">=", :"+", :"-", :"*", :"/", :"!", :"~", :"%", :"&", :"|", :"^", :"**", :"[]", :"[]=", :"<=>", :"[]?", :"&+", :"&-", :"&*", :"&**"]
DefOrMacroCheck2 = [:"<<", :"<", :"<=", :"==", :"===", :"!=", :"=~", :"!~", :">>", :">", :">=", :"+", :"-", :"*", :"/", :"!", :"~", :"%", :"&", :"|", :"^", :"**", :"[]", :"[]?", :"[]=", :"<=>", :"&+", :"&-", :"&*", :"&**"]

def parse_def_helper(is_abstract = false)
push_def
Expand Down