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

Remove unary + from the type system #5514

Merged
merged 2 commits into from
Dec 6, 2018
Merged

Conversation

corollari
Copy link
Contributor

@corollari corollari commented Nov 27, 2018

Description

Remove unary + from the type system, as requested in #5470

Fixes #5470

Checklist

  • Code compiles correctly
  • All tests are passing
  • New tests have been created which fail without the change (if possible)
  • README / documentation was extended, if necessary
  • Changelog entry (if change is visible to the user)
  • Used meaningful commit messages

Not sure if any new test is needed, feedback would be appreciated.

@stackenbotten
Copy link

There was an error when running test_check_style for commit b51dd283de7bcbdeda85ebd78c7729a09e86d269:

Error: Trailing whitespace found:
 libsolidity/ast/Types.cpp:616: else if (_operator == Token::Sub || _operator == Token::Inc || 

Please check that your changes are working as intended.

@leonardoalt
Copy link
Member

leonardoalt commented Nov 27, 2018

The unary operator tests now return:

SyntaxError: (70-72): Use of unary + is disallowed.
  TypeError: (70-72): Unary operator + cannot be applied to type uint256

If the type system disallows it, can we remove it from the syntax checker?
@chriseth @axic @bit-shift

@chriseth
Copy link
Contributor

@leonardoalt I would first like to see the full test updates without removal from the syntax checker.

@corollari corollari force-pushed the develop branch 3 times, most recently from 979a47a to f6767a6 Compare November 28, 2018 12:23
@chriseth
Copy link
Contributor

Please also update unary_operators in SolidityExpressionCompiler.cpp

@corollari
Copy link
Contributor Author

Working on it

@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #5514 into develop will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5514      +/-   ##
===========================================
- Coverage    88.12%   88.12%   -0.01%     
===========================================
  Files          324      324              
  Lines        31955    31954       -1     
  Branches      3828     3828              
===========================================
- Hits         28161    28160       -1     
  Misses        2498     2498              
  Partials      1296     1296
Flag Coverage Δ
#all 88.12% <100%> (-0.01%) ⬇️
#syntax 28.91% <100%> (-0.01%) ⬇️

Remove unary + from the type system
@corollari
Copy link
Contributor Author

Done

@chriseth
Copy link
Contributor

chriseth commented Dec 6, 2018

Thanks a lot!

@chriseth chriseth changed the title Fix #5470 - Remove unary + from the type system Remove unary + from the type system Dec 6, 2018
@chriseth chriseth merged commit 5fde279 into ethereum:develop Dec 6, 2018
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

4 participants