Skip to content

Conversation

Marenz
Copy link
Contributor

@Marenz Marenz commented Aug 26, 2021

fixes #11469

@Marenz
Copy link
Contributor Author

Marenz commented Aug 26, 2021

The test is failing and I am not sure why:

semanticTests/enums/minmax.sol: FAIL
  Contract:
    contract test {
        enum MinMax { A, B, C, D }

        function min() public returns(uint) { return uint(type(MinMax).m
in); }
        function max() public returns(uint) { return uint(type(MinMax).m
ax); }

    }

    // ====
    // compileViaYul: also

  Expected result:
  // min() -> 0
  // max() -> 4

  Obtained result:
  // min() -> 0
  // max() -> FAILURE, hex"4e487b71", 0x21
  Warning: The call to "max()" returned
  [4e,48,7b,71]
  [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,21]

Any clues are welcome!

unsigned int memberValue(ASTString const& _member) const;
size_t numberOfMembers() const;
unsigned int minValue() const { return 0; }
unsigned int maxValue() const { return static_cast<unsigned int>(numberOfMembers()); }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Max amount of members is 256 so this is fine

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to subtract 1 from the size ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Max amount of members is 256 so this is fine

Even then this should have an assert so that we find this in case the limit changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You need to subtract 1 from the size ;)

Hmm must have forgotten to push that, my local version has it already

@Marenz Marenz force-pushed the enum-min-max-11469 branch from 58999f7 to 8771e93 Compare August 26, 2021 16:23
@Marenz Marenz force-pushed the enum-min-max-11469 branch 2 times, most recently from a6d5c86 to 154f53f Compare August 30, 2021 10:09
@Marenz Marenz marked this pull request as ready for review August 30, 2021 10:10
@Marenz Marenz force-pushed the enum-min-max-11469 branch from 154f53f to 84c80ee Compare August 30, 2021 11:04
@chriseth
Copy link
Contributor

Please add changelog entry and documentation.

@Marenz Marenz force-pushed the enum-min-max-11469 branch from 84c80ee to c36df1a Compare August 31, 2021 12:40
Changelog.md Outdated
### 0.8.8 (unreleased)

Language Features:
* Type System: Add ``type().min`` and ``type().max`` for enums
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Type System: Add ``type().min`` and ``type().max`` for enums
* Type System: Suppert ``type().min`` and ``type().max`` for enums.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suppert :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did add the right spelling though :P

@Marenz Marenz force-pushed the enum-min-max-11469 branch 2 times, most recently from 1fa0abd to 60db3cc Compare August 31, 2021 12:49
@Marenz Marenz force-pushed the enum-min-max-11469 branch from 60db3cc to df18075 Compare August 31, 2021 15:39
@Marenz Marenz force-pushed the enum-min-max-11469 branch from df18075 to 0608761 Compare September 1, 2021 11:04
Copy link
Collaborator

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Looks good apart from the dot ;-).

@Marenz Marenz force-pushed the enum-min-max-11469 branch from 0608761 to 2b28f87 Compare September 1, 2021 13:02
Copy link
Collaborator

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

LGTM.

@chriseth chriseth enabled auto-merge September 1, 2021 13:21
@chriseth chriseth merged commit 29cc7aa into develop Sep 1, 2021
@chriseth chriseth deleted the enum-min-max-11469 branch September 1, 2021 13:43
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.

Add type().length for enums

6 participants