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

add checker error when non-explicitly downcasting integers internally #23695

Open
benduckwdesign opened this issue Feb 12, 2025 · 0 comments
Open
Labels
Bug This tag is applied to issues which reports bugs. Unit: JS Bugs/feature requests, that are related to the JavaScript backend.

Comments

@benduckwdesign
Copy link

benduckwdesign commented Feb 12, 2025

Describe the bug

spytheman helped me debug this issue.
The function signature of string_at (alphabet[remainder]) is u8 string_at(string s, int idx), where int is 32bit (as disclosed in the V docs).
The C backend silently downcasts u64 to int in order to match the function type signature of the underlying builtin string function. But it is not possible to do this for the JS backend, which will give an error.

Reproduction Steps

fn u64_to_custom_base(n u64) string {
    alphabet := "abcdefghijklmnopqrstuvwxyz0123456789"
    alpha_len := u64(alphabet.len)
    mut result := ''
    
    if n == 0 {
        return alphabet[0].ascii_str()
    }
    
    mut num := n
    for num > 0 {
        remainder := num % alpha_len
        result = alphabet[remainder].ascii_str() + result
        num = num / alpha_len
    }
    
    return result
}

fn main() {
    println(u64_to_custom_base(69))
}
> v -b js_browser .
> node .\xxxxx.js 
xxxxx.js:7541
                                result = new string(new string( u8_ascii_str(new u8(alphabet.str.charCodeAt(remainder)),).valueOf() + result.valueOf()));
                                                                                                       ^

TypeError: Cannot convert a BigInt value to a number
    at String.charCodeAt (<anonymous>)
    at main__BaseConverter_u64_to_custom_base (xxxxx.js:7541:76)
    at main__DocTree_render (xxxxx.js:8453:51)
    at js_main (xxxxx.js:7714:21)
    at xxxxx:9300:25

Node.js v20.13.0

Expected Behavior

Expected an error during compile time, alternatively for JS to downcast the u64 to int and print base-encoded string.

Current Behavior

V generates JS and C code for this without any warnings or errors. The code works in C but not in JS.

Possible Solution

spytheman suggests this should be a checker error so that it is caught earlier and does not rely strictly on backend behavior.
Personally, I agree since it might cause unexplained and unintended consequences later that the user might not be aware of.
However, I'm not against preventing this conversion even without it being explicitly listed on the conversion chart

  i8 → i16 → int → i64
                  ↘     ↘
                    f32 → f64
                  ↗     ↗
   u8 → u16 → u32 → u64 ⬎
      ↘     ↘     ↘      ptr
   i8 → i16 → int → i64 ⬏

It would be nice if it threw an error only when it's non-explicit, i.e. alphabet[remainder] and not alphabet[int(remainder)]. I'm also not sure if its planned for int to always be 32 bits, if its not, it might be nice to have a compile time pseudo variable for @INT_SIZE to forward-proof conversion behavior i.e. no need to downcast if the underlying function later uses a 64bit integer.

Additional Information/Context

No response

V version

V 0.4.9 b94da8a

Environment details (OS name and version, etc.)

V full version V 0.4.9 0632822.b94da8a
OS windows, Microsoft Windows 10 Pro 19045 64-bit
Processor 24 cpus, 64bit, little endian, AMD Ryzen 9 3900XT 12-Core Processor
Memory 55.9GB/127.91GB
V executable
V last modified time 2025-02-11 01:20:52
Git version git version 2.45.1.windows.1
V git status weekly.2025.06-27-gb94da8a6 (11 commit(s) behind V master)
.git/config present true
cc version N/A
gcc version gcc (x86_64-posix-seh-rev0, Built by MinGW-Builds project) 14.2.0
clang version swift.org clang version 16.0.0
msvc version N/A
tcc version tcc version 0.9.27 (x86_64 Windows)
tcc git status thirdparty-windows-amd64 b425ac82
emcc version N/A
glibc version N/A

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.

@benduckwdesign benduckwdesign added the Bug This tag is applied to issues which reports bugs. label Feb 12, 2025
@felipensp felipensp added the Unit: JS Bugs/feature requests, that are related to the JavaScript backend. label Feb 12, 2025
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: JS Bugs/feature requests, that are related to the JavaScript backend.
Projects
None yet
Development

No branches or pull requests

2 participants