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

Conversions revamped #11284

Open
axic opened this issue Apr 21, 2021 · 2 comments
Open

Conversions revamped #11284

axic opened this issue Apr 21, 2021 · 2 comments
Labels
high impact Changes are very prominent and affect users or the project in a major way. language design :rage4: Any changes to the language, e.g. new features medium effort Default level of effort must have Something we consider an essential part of Solidity 1.0. needs design The proposal is too vague to be implemented right away

Comments

@axic
Copy link
Member

axic commented Apr 21, 2021

Currently we support (an ever shrinking number of) implicit type conversions and some explicit ones. The explicit conversions mostly truncate/extend, and only a single one (integer to enum) is performing a check and panic.

During #9170 we have endlessly debated whether converting from a larger bytes should truncate or throw a panic, and finally realised we may want to review how conversions work in general. It seems that we want to make in any way ambiguous conversions explicitly understandable.

In the past we discussed new casting/conversion syntax here and here:

  • We could consider C++ism cast<byte[]>(cast<byte[32]>(cast<uint256>(1234))) to make it worse (or use convert<>)
  • And add copyof on top of it: cast<byte[]>(copyof cast<byte[32]>(cast<uint256>(1234)))
  • uint256(1234).as<byte[32]>().as<byte[]>() may be a bit more readable :)
  • I thought we had an issue proposing a syntax something like cast<toType>(val) instead of explicit conversions. I guess it is just more verbose so people would dislike it?
  • But at least would probably be easier to read cast<address payable>(..) than address payable(..).
  • If we do have cast<> as an unchecked casting, then we could later consider adding convert<> which does checks.
  • I wonder if we could use the cast syntax for multiple-step casting, i.e. cast<uint128,int128,int16>(..uint256 input..) or cast<uint128 -> int128 -> int16>(..uin256 input..) (similar to the mapping syntax)
  • But the other option we briefly discussed is explicit casting works like static_cast/reinterpret_cast and we could introduce truncation/conversion helper on the types, i.e. bytes32.truncateFrom(<dynamic bytes>).
    -Instead of the multiple step casting I suppose there could be nicer helpers in the stdlib doing that?

To summarise, the following syntactical ideas were proposed:

  • cast<to>(from)
  • convert<to>(from)
  • from.as<to>()
  • cast<intermediate1,intermediate2,final>() (in case of a series of conversions)
  • bytes32.truncateFrom(<dynamic bytes>)
  • truncate<..>()
  • extend<..>()

On todays design call there seemed consensus to rather have explicitly understandable function names (such as truncate and extend), instead of a generic cast.

(Moved from #9170 (comment))

@axic axic added the language design :rage4: Any changes to the language, e.g. new features label Apr 21, 2021
@axic
Copy link
Member Author

axic commented Apr 21, 2021

Following up the above summary, here's an actual suggestion to get us started. While I like the member functions .as<>(), truncate<>(),.. quite a bit, it may not be the best, unless we are happy to make those function names reserved. So my second option is the following:

  1. as for "safe" conversion (e.g. contract -> address, address -> payable address)
  • as<address payable>(my_address)
  • as<address>(my_token)
  1. as for unsafe conversions, where a runtime check is generated
  • as<bytes32>(keccak256("")) -- this will work an return the empty hash
  • as<bytes32>(bytes.concat(keccak256(""), keccak256(""))) -- this will panic
  • as<bytes>(bytes32(hex"1234")) -- this will work as bytes32 will always fit
  • as<bytes8>(bytes6(hex"1234")) -- this will work as bytes6 < bytes8 and will be zero padded on the right
  • as<bytes8>(bytes10(hex"1234")) -- this will panic
  1. truncate for (potentially) truncating conversions
  • truncate<bytes32>(bytes.concat(keccak256(""), keccak256(""))) -- this will take the first 32 bytes of the input
  • truncate<bytes8>(bytes10(hex"1234")) -- this will take the first 8 bytes of the input
  1. extend for (potentially) extending conversions
  • extend<bytes8>(bytes6(hex"1234")) -- this will zero pad the input on the right

In this model we allow the old-style explicit conversion to act as "constructors", i.e. bytes32(hex"1234").

(The above is WIP and will need to updateed with conversions on numbers, fiexd points, etc. Could also decide to use something other than as for the runtime check version, i.e. try<> or convert<>)

@chriseth
Copy link
Contributor

For me, as sounds like it would not trigger runtime errors.
By the way, for templates, the parser probably requires a syntax like as.<bytes8>(...).

@cameel cameel added medium effort Default level of effort high impact Changes are very prominent and affect users or the project in a major way. must have Something we consider an essential part of Solidity 1.0. needs design The proposal is too vague to be implemented right away labels Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high impact Changes are very prominent and affect users or the project in a major way. language design :rage4: Any changes to the language, e.g. new features medium effort Default level of effort must have Something we consider an essential part of Solidity 1.0. needs design The proposal is too vague to be implemented right away
Projects
None yet
Development

No branches or pull requests

3 participants