Add binary number literals to solidity

[pcw109550]

Feature Description

Added binary number literal to solidity. Below works.

contract Test {
    bytes1 constant a = 0b01011010;
    function binaryTest() public {
        uint b = 0b101;
        uint c = 0b10101_101_10101;
        uint256 d = 0b1011 * 1 days;
        bytes4 e = 0b0; // zero value only allowed for implicit conversion
        // bytes2 f = 0b000011110000111100001111; // error, rvalue too big
        bytes3 g = 0b1111000011111111_000_00000;
        // uint16 h = 0b101__111; // error, Only one consecutive underscores between digits allowed
        // bytes6 i = 0b101; // error, Type int_const 5 is not implicitly convertible to expected type bytes6
    }
}

Syntax rules referred from hex number literals. Please check ANTLR rules and docs for exact behavior.

Tests

Mirrored all hex related tests, and added binary number literal related test if necessary. Please suggest if i missed anything.

Design

I manually implemented parsing binary strings to u256. Currently boost's multi precision lib, which is the dependency for solidity does not support binary parsing. Solidity internally uses u256 type, so I parsed manually.

New Errors

5146_error is introduced.

	if (_literal.isBinNumber() && _literal.subDenomination() != Literal::SubDenomination::None)
		m_errorReporter.fatalTypeError(
			5146_error,
			_literal.location(),
			"Binary numbers cannot be used with unit denominations. "
			"You can use an expression of the form \"0b1011 * 1 days\" instead."
		);

[github-actions]

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

[nikola-matic]

@ekpyron @cameel we do the same error for hex numbers as well, and this error is kinda trash; can't we just spew out an 'invalid hex/binary literal' here?

[cameel]

Yeah, the message could be better, but just invalid hex/binary literal seems too vague to me. It should still say what exactly is wrong (i.e. that you can't randomly stick an identifier at the end of this number without any intervening whitespace).

[nikola-matic]

Suggested change

	// SyntaxError 2990: (56-87): Invalid use of underscores in number literal. Only one consecutive underscores between digits allowed.
	// SyntaxError 2990: (56-87): Invalid use of underscores in number literal. Only one consecutive underscore between digits is allowed.

Don't do anything about this (at least not in this PR), I'm just putting this as a reminder here :) Of course, if you wanna open another PR to address this, by all means.

[pcw109550]

Gotcha. If I open a additional PR to fix this, and if the new PR gets merged first, I shall rebase and fix this as well.

[pcw109550]

Opened additional PR at #15049.

[pcw109550]

additional PR is merged to develop so rebased this PR, and fixed the error msg at 862aa9f.

[nikola-matic]

Please add some syntax/semantic tests for literal arithmetic, i.e. 0b01 + 0b10, etc.

[pcw109550]

May I ask writing semantic tests like

contract test {
    function add() public pure returns (uint c) {
        c = 0b01 + 0b110;
    }
    function sub() public pure returns (uint c) {
        c = 0b0111110 - 0b110;
    }
    function mul() public pure returns (uint c) {
        c = 0b110 * 0b01011;
    }
    function div() public pure returns (uint c) {
        c = 0b10111 / 0b11;
    }
    function mod() public pure returns (uint c) {
        c = 0b110101010 % 0b1011;
    }
}
// ----
// add() -> 0x7
// sub() -> 0x38
// mul() -> 0x42
// div() -> 0x26
// mod() -> 0x8

is the right way? If it is, should I go for every binary / unary operators, including +,-,*,/,%,^,%? How should the test be complex?

Also, can you recommend any existing syntax tests that we should add more, or any examples? I do not get the concrete idea for adding new syntax tests; compared to semantic test.

[pcw109550]

Can you please point some existing tests that tests literal arithmetic, like 0x01 + 0x10 or 1 + 2? I agree that adding tests for binary number literal is necessary because it is a new feature, but if those tests exists, referencing them would be helpful for me to add more tests.

[nikola-matic]

The above is fine; I doubt we have many such tests, since it's been a very long time since we've introduced a feature like this; you can take a look at semanticTests/arithmetics, semanticTests/exponentiation, and probably some more, but as I said, what you have here should be enough.

[pcw109550]

I have added two tests at 7cdaa37.

[nikola-matic]

Hi @pcw109550, we had another call today where we discussed this feature (PR), and decided that we ultimately won't be merging it. @ekpyron explained it in the public channel on matrix here:

The underlying issue for that PR wasn't closed without reason: we've not established that this will make for a worthwhile extension of the language. We generally need to be careful when introducing features to the language, since they will require support for compilation directly (for which the PR likely does fine), but also support from other components of the compiler (like the SMTChecker) and from tooling - furthermore any future revision of the language will need to account for this as well forever - those are ongoing costs, since we won't be able to easily remove features again after being introduced. That's why we redirected the issue to the forum, s.t. we could revisit it, if and only if there is strong demand for it, which hasn't exactly manifested there either. We probably should have been clearer about that from the start, sorry about that, but it's always better to confirm with us before starting to implement features.

I'm really sorry, since the PR itself is great work and does implement the feature properly - i.e. the quality of your work is not being questioned, rather, the feature itself does not really seem to have strong demand from the community, and it would set a fairly bad precedent if were to allow merging of features that have not been decided as necessary/wanted.

Again, apologies for the inconvenience, and I hope this won't deter you from contributing further to Solidity, just make sure to ask before starting implementation whether the feature you want to work on is needed at the moment.