- 
                Notifications
    You must be signed in to change notification settings 
- Fork 192
          gccrs: Fix negative bounds in RangePattern being handled incorrectly
          #4243
        
          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
base: master
Are you sure you want to change the base?
Conversation
RangePattern being handled incorrectly
      0d81649    to
    c434d0c      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think LGTM but not sure how it works now with the has_minus removed
…sitive
GIMPLE output for compile/issue-4242.rs:
...
  x = 1;
  RUSTTMP.2 = x;
  _1 = RUSTTMP.2 >= -55;
  _2 = RUSTTMP.2 < 0;
  _3 = _1 & _2;
  if (_3 != 0) goto <D.112>; else goto <D.113>;
  <D.112>:
  {
    RUSTTMP.1 = 2;
    goto <D.105>;
  }
  <D.113>:
  _4 = RUSTTMP.2 >= -99;
  _5 = RUSTTMP.2 < -55;
  _6 = _4 & _5;
  if (_6 != 0) goto <D.114>; else goto <D.115>;
  <D.114>:
  {
    RUSTTMP.1 = 3;
    goto <D.105>;
  }
...
gcc/rust/ChangeLog:
	* backend/rust-compile-pattern.cc (compile_range_pattern_bound): Set litexpr
	to negative if has_minus is present in the RangePatternBoundLiteral param.
Signed-off-by: Yap Zhi Heng <[email protected]>
    c434d0c    to
    2f7aa77      
    Compare
  
    2f7aa77    to
    9e2c020      
    Compare
  
    Checks whether upper bound of range is not lower or equal to the lower bound. gcc/rust/ChangeLog: * backend/rust-compile-pattern.cc(compilePatternCheckExpr::visit(RangePattern)): Add E0579 check to ensure that lower bound is always below upper bound. Signed-off-by: Yap Zhi Heng <[email protected]>
9e2c020    to
    8ee2b15      
    Compare
  
    |  | ||
| rust_assert ( | ||
| (TREE_CODE (upper) == REAL_CST && TREE_CODE (lower) == REAL_CST) | ||
| || (TREE_CODE (upper) == INTEGER_CST && TREE_CODE (lower) == INTEGER_CST)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does 1f32..1.2f32 break anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Triggers an error that shouldn't be triggered lol, maybe we can create a new issue for it.
error: mismatched types, expected ‘f32’ but got ‘<integer>’ [E0308]
    7 |         1f32..1.2f32 => 1,
      |         ^~~~Anyways, 1.0f32..1.2f32 compiles properly and does not trigger E0579 but -1.2f32..-1.0f32 triggers E0579... not sure what went wrong here :/
Fixes #4242
Fixes #3659
The ICE for #3659 was initially fixed by #4049, but the test case did not throw E0579 as expected. Moreover, the output GIMPLE was found to be wrong (which led to the creation of #4242). This PR aims to fix both issues by correcting parser behavior for
RangePattern-s and adding E0579 error checking.