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

Match fails oddly if branch is changed from int to i64 #21624

Closed
mike-ward opened this issue Jun 1, 2024 · 5 comments · Fixed by #21696
Closed

Match fails oddly if branch is changed from int to i64 #21624

mike-ward opened this issue Jun 1, 2024 · 5 comments · Fixed by #21696
Labels
Bug This tag is applied to issues which reports bugs. Unit: Type System Bugs/feature requests, that are related to the V types system.

Comments

@mike-ward
Copy link
Contributor

mike-ward commented Jun 1, 2024

V doctor:

V full version: V 0.4.6 8215f21.8c24936
OS: macos, macOS, 14.4.1, 23E224
Processor: 8 cpus, 64bit, little endian, Apple M2

getwd: /Users/mike/Documents/github/coreutils/src/bug
vexe: /Users/mike/Documents/github/v/v
vexe mtime: 2024-06-01 11:41:36

vroot: OK, value: /Users/mike/Documents/github/v
VMODULES: OK, value: /Users/mike/.vmodules
VTMP: OK, value: /tmp/v_501

Git version: git version 2.45.1
Git vroot status: 0.4.6-64-g8c249366
.git/config present: true

CC version: Apple clang version 15.0.0 (clang-1500.3.9.4)
thirdparty/tcc status: thirdparty-macos-arm64 5c1d002f

What did you do?
v bug_test.v

bug.v

// convert strings like 10K to i164
const block = 512
//const block = i64(512)

// **1
const kilo = i64(1024)
const kilobyte = i64(1000)

// **2
const mega = kilo * kilo
const megabyte = kilobyte * kilobyte

// **3
const giga = mega * kilo
const gigabyte = megabyte * kilobyte

// **4
const terra = giga * kilo
const terrabyte = gigabyte * kilobyte

// **5
const peta = mega * kilo
const petabyte = megabyte * kilobyte

// **6
const exa = peta * kilo
const exabyte = peta * kilo

// **7
const zetta = exa * kilo
const zettabyte = exabyte * kilobyte

// **8
const yotta = zetta * kilo
const yottabyte = zettabyte * kilobyte

// **9
const ronna = yotta * kilo
const ronnabyte = yottabyte * kilobyte

// **10
const quetta = ronna * kilo
const quettabyte = ronnabyte * kilobyte

fn string_to_i64(s string) ?i64 {
	if s.len == 0 {
		return none
	}

	mut index := 0
	for index < s.len {
		match true {
			s[index].is_digit() {}
			s[index] == `+` && index == 0 {}
			s[index] == `-` && index == 0 {}
			else { break }
		}
		index += 1
	}

	number := s[0..index].i64()
	suffix := if index < s.len { s[index..] } else { 'c' }

	multiplier := match suffix.to_lower() {
		'b' { block }
		'k' { kilo }
		'kb', 'kib' { kilobyte }
		'm' { mega }
		'mb', 'mib' { megabyte }
		'g' { giga }
		'gb', 'gib' { gigabyte }
		't' { terra }
		'tb', 'tib' { terrabyte }
		'p' { peta }
		'pb', 'pib' { petabyte }
		'e' { exa }
		'eb', 'eib' { exabyte }
		'z' { zetta }
		'zb', 'zib' { zettabyte }
		'y' { yotta }
		'yb', 'yib' { yottabyte }
		'r' { ronna }
		'rb', 'rib' { ronnabyte }
		'q' { quetta }
		'qb', 'qib' { quettabyte }
		// oddball formats found in __xstrtol source
		'c' { 1 }
		'w' { 2 }
		else { return none }
	}

	result := number * multiplier
	if result == 0 && number != 0 {
		return none
	}
	return result
}

bug_test.v

module main

fn test_string_to_i64_conversions() {
	assert string_to_i64('19P')! == 19 * peta
	assert string_to_i64('18T')! == 18 * terra
}

What did you expect to see?
bug_test.v should pass

What did you see instead?

failed propagation with error:
    5 |         assert string_to_i64('18T')! == 18 * terra

Now change the second line in bug.v from

const block = 512

to

const block = i64(512)

and the test passes.

This only happens with the t branch in the code. Other branches work: (e.g. assert string_to_164('10P')! == 10 * peta)

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@felipensp
Copy link
Member

Yes. Because the first branch on Match is instructing that the return type is int not i64.

You can use: 'b' { i64(block) } to force i64 too.

assert string_to_i64('18T')! == 18 * terra

With int:
number: 18
multiplier: 0
result: 0

With i64:
number: 18
multiplier: 1099511627776
result: 19791209299968

@mike-ward
Copy link
Contributor Author

mike-ward commented Jun 1, 2024

Yes. Because the first branch on Match is instructing that the return type is int not i64.

Why does the p branch work?

Also, I would expect multiplier to always be i64. Otherwise, the other branches should be errors since they are i64

@mike-ward
Copy link
Contributor Author

Why does the p branch work?

Ah, I defined peta wrong. bummer.

Still seems odd that match does not issue an error in the other branches beause of assignment of an i64 to int

@mike-ward
Copy link
Contributor Author

For my own understanding:

Why doesn't V report an error when the first branch is an int and the other branches are i64?

The current behavior suggests that V is silently casting the 2nd and subsequent branches to int.

@spytheman
Copy link
Member

spytheman commented Jun 4, 2024

I agree. Auto promoting int to i64 is fine, since it an int fits entirely in the range of i64, but automatically casting i64 to int implicitly, should not be allowed.

@spytheman spytheman added Bug This tag is applied to issues which reports bugs. Unit: Type System Bugs/feature requests, that are related to the V types system. labels Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs. Unit: Type System Bugs/feature requests, that are related to the V types system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants