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

docs: fix errors in docs #7801

Merged
merged 1 commit into from
Jan 4, 2021
Merged

docs: fix errors in docs #7801

merged 1 commit into from
Jan 4, 2021

Conversation

tkshnwesper
Copy link
Contributor

Hello V community,

I've made two changes:

  1. Removed arrow pointing from int to f32 because as the documentation says that conversion cannot be made (unless I have misunderstood it):

An int value for example can be automatically promoted to f64
An int value for example can be automatically promoted to f64
or i64 but not to f32 or u32.

  1. Fixed a grammatical error by adding an "if".

Cheers!

@spytheman
Copy link
Member

fn main() { 
   a := int(12)
   mut f := f32(0)
   f = a
   println(f)
}

this compiles cleanly and prints: 12.

@JalonSolov
Copy link
Contributor

It's not a question of whether or not some ints can be cast to f32, it is whether or not all ints can be cast to f32. They can't, because f32 only handles up to 7 digit integers, where an int can be up to 10 digits. If you try to store all ints in an f32, you lose precision as the number gets higher.

@tkshnwesper
Copy link
Contributor Author

We could either remove the arrow or mention that not all ints can be converted to f32. But the latter might confuse users so we would have to include an example of what can and cannot be converted successfully. However, to keep things simple it might be better to just remove the arrow.

@UweKrueger
Copy link
Member

UweKrueger commented Jan 3, 2021

... f32 only handles up to 7 digit integers, where an int can be up to 10 digits. If you try to store all ints in an f32, you lose precision ...

That's right and that is why in #4971 and #5155 I had originally added checks that did forbid int -> f32. However there was a discussion and the result was that I had to relax this again in #5174. So now int -> f32 is allowed, again.

The philosophy now is somehow: Conversion is allowed if the result fits in the limits of the target type, but we may lose (some) precision.
i64 -> f32 is still forbidden, but I agree that there is no real logical reason for this - it's more that the loss in precision is not balanced by the gain in range...

@tkshnwesper
Copy link
Contributor Author

It's possible to convert int to u32 or even i16 if the int is small enough. However, the documentation does not have arrows going from the int to u32 or i16.

It might be a good idea to define what the arrow and it's direction means, for types a and b if we have a -> b then which of the following should apply?

  1. All of a can be converted to b without losing precision
  2. a can be converted to b without compilation error
  3. Some of a can be converted to b

From your point @UweKrueger,

The philosophy now is somehow: Conversion is allowed if the result fits in the limits of the target type, but we may lose (some) precision.

I gather it to be 2 and 3. So either we should add bidirectional arrows <-> for types like i16 and int, or if backward conversion should not be allowed by nature, then it should result in a compilation error.

However, if 1 and 2 are the correct ones, then we should remove the arrow.

@medvednikov
Copy link
Member

I myself still can't make the decision.

I incline to believe @dumblob is right and that commit that I asked you to push @UweKrueger has to be reverted.

But I still need to think about it and do research.

@UweKrueger
Copy link
Member

UweKrueger commented Jan 3, 2021

Yes it's a difficult decision. But I think #5174 should be reverted, too.
There is also Issue #7692 about this topic.

@dumblob
Copy link
Contributor

dumblob commented Jan 3, 2021

The question is always what the default behavior should be. It might be less difficult to decide if we introduced some new arguments like -floats_imprecise etc. (or fn attribute [floats_imprecise]) which would be less strict but would allow to write more performant code.

So, as you already noticed, I always prefer correct and well defined behavior on all supported platforms by default.

@spytheman spytheman marked this pull request as draft January 4, 2021 17:06
@spytheman
Copy link
Member

In all cases, the documentation should match the code, otherwise it is misleading to the reader.
Currently, the arrows should stay, because that code does compile.
In the future that may change.

@tkshnwesper
Copy link
Contributor Author

@spytheman Alright, I've kept the arrows intact.

@spytheman spytheman marked this pull request as ready for review January 4, 2021 19:53
@spytheman spytheman merged commit d5b510d into vlang:master Jan 4, 2021
@tkshnwesper tkshnwesper deleted the patch-1 branch January 5, 2021 04:51
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.

6 participants