[ruby-core:123681] [Ruby Bug#21669] Thoroughly implement void value expression check
Issue #21669 has been reported by mame (Yusuke Endoh). ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Eregon (Benoit Daloze). This check is getting more and more complicated and I wonder of the value of having it. In my opinion there is little value of have this check, so given the complexity and the cost to check it (needs extra traversals of potentially large parts of the AST) I would just remove the check entirely. Has this check ever been useful to anyone? ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115065 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Eregon (Benoit Daloze). The whole concept of "void value expression" is also weird in Ruby, where every expression/statement has a value, so I think it feels like something that doesn't belong to Ruby. It probably only triggers for pretty broken code, and even then not emitting the error or warning would result in correct execution, just some dead code (the assignment) would always be not executed. Dead code can't be detected easily in Ruby, so why bother for such a narrow case that probably no one runs into? ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115066 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Earlopain (Earlopain _). In prism, that whole check fits in about 130 lines: https://github.com/ruby/ruby/blob/05deec695df55bb72bae8aace673dd2926729d39/s.... Looking at that function, probably it would not be very difficult for prism to support this new behaviour. Not so much complexity, but the runtime cost is there of course (I haven't measured it, I just know it's not zero). I do believe this check has value. Consider `return 1 and 2`, which it currently rejects and also highlights that because of `and` precedence this basically becomes `(return 1) and 2`. That said, the cases here are certainly contrived. ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115067 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by mame (Yusuke Endoh). Eregon (Benoit Daloze) wrote in #note-1:
This check is getting more and more complicated and I wonder of the value of having it.
IMO, the current is indeed complicated because it is inconsistent. I think the new behavior is consistent and thus simpler. At least it's explainable. I am a bit concerned about compatibility, though. Earlopain (Earlopain _) wrote in #note-3:
In prism, that whole check fits in about 130 lines
As an aside, I noticed this inconsistency when Coverity Scan, a static analyzer for C, pointed out that the following branch in Prism was dead code: https://github.com/ruby/prism/blob/2ecd49dfeea1d0b00c3af609be82dd488d25c7f3/... At line 1067, `void_node = vn` is executed (and `vn` is guaranteed to be non-NULL on the line above), so it's impossible for `void_node` to be NULL at line 1075. Thus, it is indeed dead code. To remove the dead code, I started writing some test codes, and found these various inconsistencies. Incidentally, it wasn't a bug in Prism, but rather the result of perfectly replicating the inconsistent behavior in parse.y. ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115069 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by kddnewton (Kevin Newton). I agree with @mame — I think it is valuable, especially when it will be consistent. Happy to implement this change in prism. ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115088 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Earlopain (Earlopain _). I have an implementation in https://github.com/ruby/prism/pull/3728 (maybe needs more tests) How about waiting on Ruby 4.1 for this? We can then introduce it early in the development cycle to find potential compatibility concerns. preview2 for ruby 4.0 has already released, I feel it is a bit late right now. ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115246 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by nobu (Nobuyoshi Nakada). Sorry, I've missed this issue. It is done to fix parse.y for 1 and 2. I think 1 is OK, and probably no problem in practice. But 2 is intentionally passed, IIRC, and bootraptest has such code. ```ruby #1526 test_syntax.rb:572: 1.times{ p(1, (next; 2)) }; :ok #=> "" (expected "ok") stderr output is not empty bootstraptest.test_syntax.rb_572_1526.rb: bootstraptest.test_syntax.rb_572_1526.rb:5: void value expression (SyntaxError) p(1, (next; 2)) ^~~~ #1528 test_syntax.rb:584: i = 0 1 + (while true break 2 if (i+=1) > 1 p(1, (next; 2)) end) #=> "" (expected "3") stderr output is not empty bootstraptest.test_syntax.rb_584_1528.rb: bootstraptest.test_syntax.rb_584_1528.rb:7: void value expression (SyntaxError) p(1, (next; 2)) ^~~~ #1529 test_syntax.rb:592: i = 0 1.times{ break if i>1 i+=1 p(1, (redo; 2)) }; :ok #=> "" (expected "ok") stderr output is not empty bootstraptest.test_syntax.rb_592_1529.rb: bootstraptest.test_syntax.rb_592_1529.rb:8: void value expression (SyntaxError) p(1, (redo; 2)) ^~~~ #1531 test_syntax.rb:607: i = 0 1 + (while true break 2 if (i+=1) > 1 p(1, (redo; 2)) end) #=> "" (expected "3") stderr output is not empty bootstraptest.test_syntax.rb_607_1531.rb: bootstraptest.test_syntax.rb_607_1531.rb:7: void value expression (SyntaxError) p(1, (redo; 2)) ^~~~ #259 test_flow.rb:374: $a = []; begin; ; $a << 1 def m1 *args; $a << 2 ; $a << 3 end; $a << 4 def m2; $a << 5 m1(:a, :b, (return 1; :c)); $a << 6 end; $a << 7 m2; $a << 8 ; $a << 9 ; rescue Exception; $a << 99; end; $a #=> "" (expected "[1, 4, 7, 5, 8, 9]") stderr output is not empty bootstraptest.test_flow.rb_374_259.rb: bootstraptest.test_flow.rb_374_259.rb:8: void value expression (SyntaxError) m1(:a, :b, (return 1; :c)); $a << 6 ^~~~~~~~ #261 test_flow.rb:395: $a = []; begin; ; $a << 1 def m2; $a << 2 end; $a << 3 def m(); $a << 4 m2(begin; $a << 5 2; $a << 6 ensure; $a << 7 return 3; $a << 8 end); $a << 9 4; $a << 10 end; $a << 11 m(); $a << 12 ; $a << 13 ; rescue Exception; $a << 99; end; $a #=> "" (expected "[1, 3, 11, 4, 5, 6, 7, 12, 13]") stderr output is not empty bootstraptest.test_flow.rb_395_261.rb: bootstraptest.test_flow.rb_395_261.rb:10: void value expression (SyntaxError) ``` ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115467 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by mame (Yusuke Endoh). Earlopain (Earlopain _) wrote in #note-6:
I have an implementation in [https://github.com/ruby/prism/pull/3728](https://github.com/ruby/prism/pull/3728) (maybe needs more tests)
Great, thanks!
How about waiting on Ruby 4.1 for this?
Agreed. I think the evaluation window is too short to introduce this now, and considering the concern raised by nobu, we may need to make some adjustments. nobu (Nobuyoshi Nakada) wrote in #note-7:
Sorry, I've missed this issue.
Don't be sorry, thank you!
But 2 is intentionally passed, IIRC, and bootraptest has such code.
I don't think `bootstraptest` necessarily reflects real-world use cases, but I think users might write code that returns in the middle of a `begin` block, such as during debugging. This disturbs such code. @matz What do you think? ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115468 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by nobu (Nobuyoshi Nakada). Earlopain (Earlopain _) wrote in #note-6:
I have an implementation in https://github.com/ruby/prism/pull/3728 (maybe needs more tests)
It doesn't reject the following cases. ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115469 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by mame (Yusuke Endoh). Discussed this at the dev meeting. I was convinced that it is very rare to use `return` or other escapes from the begin block *whose result is used* even during debugging. ``` def foo bar(begin return 42 1 end) end ``` So Let's try the original proposal as is. We would like to merge them into the master branch towards Ruby 4.1, after Ruby 4.0.0 is released. @Earlopain Could you read nobu's comment and implement the check in prism? ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115594 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by nobu (Nobuyoshi Nakada). https://github.com/ruby/ruby/pull/15498 ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115597 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Earlopain (Earlopain _). Yeah, it seems like I missed this case but @nobu already took care of it in his PR. I left a comment about something, however since this is in ruby/ruby now I can't actually push any changes anymore myself. I ran this change over code available on rubygems and it only impacts 37 files which go syntax invalid with this. If I take out duplicate vendorered files, it is down to just 7. So indeed no one seems to actually commit such code. But perhaps it is more common during debugging. ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115652 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by jeremyevans0 (Jeremy Evans). Earlopain (Earlopain _) wrote in #note-12:
I ran this change over code available on rubygems and it only impacts 37 files which go syntax invalid with this. If I take out duplicate vendorered files, it is down to just 7.
If it's not too much work, can you please post the 7 examples? ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115655 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Earlopain (Earlopain _). Yeah, sure. I compiled them in this gist: https://gist.github.com/Earlopain/79d49df05cca5b0c8c6cefc6f3284a4b. I missed one vendored file, so there are only 6 distinct cases. In `ruby/prism` I added a script for this. You call it like `bin/compare main void-value-expression ~/all-of-rubygems/` where the first two args are git references you want to compare against and the 3rd is the folder with all the code. At the moment it generates a rather ugly report that doesn't tell you too much about what actually changed except the bare minimum (valid previously -> now invalid, etc.), so for now I manually look at the files for more details. I want to improve that in the future. ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-115657 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
Issue #21669 has been updated by Earlopain (Earlopain _). prism is done, it just needs the parse.y changes from https://github.com/ruby/ruby/pull/15498 that @nobu prepared ---------------------------------------- Bug #21669: Thoroughly implement void value expression check https://bugs.ruby-lang.org/issues/21669#change-116263 * Author: mame (Yusuke Endoh) * Status: Open * Assignee: prism * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- A void-value-expression check is a syntax check that raises a `SyntaxError` if an expression that cannot grammatically return a value (a "void value expression," such as a `return` expression) appears in a context where a value is expected (e.g., the right-hand side of an assignment, the argument of a method call, etc.). A typical example rejected by this check is `x = return`. However, it has become clear that this check is incomplete. This ticket summarizes the results of a discussion with @matz at the hackathon before RubyWorld Conference today. ## 1\. Expressions containing branches @matz said that an expression containing branches should be considered a void value expression only if it can be grammatically determined that all possible branches do not return a value. Based on this, both parse.y and prism currently reject the following code with a "void value expression" `SyntaxError`, but it should be accepted: ```ruby x = begin raise return rescue "OK" else return end # Expected: Code parses and executes successfully. # Actual: Rejected with "unexpected void value expression" SyntaxError. ``` Reason: The `rescue` clause can return a value ("OK"), so the entire `begin` expression is not void. Conversely, the following code is currently accepted by both parse.y and prism, but it should be rejected: ```ruby x = begin foo rescue return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Reason: This expression always does `return`, so it is void whether `foo` raises an exception or not. Furthermore, `case` expressions must also be rejected similarly when all branches are void: ```ruby x = case when 1; return when 2; return else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that `if` expressions appear to be implemented correctly as intended: ```ruby x = if rand < 0.5 return else return end # Expected: Rejected with SyntaxError. # Actual: Rejected. (OK!) ``` ## 2\. Statement lists containing a void value expression @matz also said that if a statements node contains a void value expression, the entire statement list should be treated as a void value expression, even if the void expression is not the last one. Therefore, the following code is currently accepted but should be rejected: ```ruby x = begin return "NG" end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` The same applies to branches within other expressions: ```ruby x = if rand < 0.5 return "NG" else return end # Expected: Rejected with SyntaxError. # Actual: Code parses and executes. ``` Note that if a return statement is inside a branch, that statement cannot be considered a void value expression, so the entire expression also cannot be a void value expression. ```ruby x = begin return if true "NG" end # Expected: Code parses and executes. # Actual: Code parses and executes as expected. (OK!) ``` --- This is considered a bug, but if it causes practical compatibility issues, it may be necessary to plan a migration path or revisit the specifications. (Both prism and parse.y need to be fixed, but I'll assign it to the prism team for now.) -- https://bugs.ruby-lang.org/
participants (6)
-
Earlopain (Earlopain _) -
Eregon (Benoit Daloze)
-
jeremyevans0 (Jeremy Evans)
-
kddnewton (Kevin Newton)
-
mame (Yusuke Endoh)
-
nobu (Nobuyoshi Nakada)