julelang / jule

Effective programming language to build efficient, fast, reliable and safe software while maintaining simplicity
https://jule.dev
BSD 3-Clause "New" or "Revised" License
128 stars 13 forks source link

About fixed with integers and PR #73 #75

Closed mertcandav closed 7 months ago

mertcandav commented 7 months ago

What would you like to share?

The PR #73 causes build errors, therefore reverted. This problem have to investigate. If problem can solved, this PR will be implemented.

Also here is a few questions for the PR author @vil02:

Relevant PRs:

Additional information

No response

vil02 commented 7 months ago
* This changes can effect performance?

No.

* This changes reduces or increases portability of code between compilers, operating systems, CPUs?

It increases portability. The size of the basic integer types is not fixed. For example the long is 64 bits in gcc and 32 bits in MSVC. Please have a look at these two examples:

As suggested here - please try to rerun the failed jobs.

mertcandav commented 7 months ago

I can rerun all failed jobs but it will not change anything I think. CI uses compiler that compiled-from-source via main branch of Jule. Probably your PR compiled because changes not merged to main. Therefore I need to investigate that and I did, as a result my local compiler generates compilation errors same as CI.

I removed explicit generics of std::make_tuple to solve problem. Clang compiles successfully, but GCC generates errors such as not declared, for example jule::I32 not declared, did you mean jule::F32. Do you know why is GCC cannot compile?

I also tried with typedef instead of using keyword. Same result.

My GCC compile command is: g++-13 --std=c++17 dist/ir.cpp

vil02 commented 7 months ago

I just realized that U32 has the same size as U64, so U32, U64, Uint and std::size_t are all the same. Therefore, #73 introduced some problems (because U32 and I32 started to have 32 bits).

mertcandav commented 7 months ago

Alright. I solved problem. GCC generates error because I missed to include <cstdint> header.

I will push your changes. Thanks for your contribution.