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

Type conversion warnings on Windows+MSVC #65

Open
eyalroz opened this issue Feb 27, 2019 · 6 comments
Open

Type conversion warnings on Windows+MSVC #65

eyalroz opened this issue Feb 27, 2019 · 6 comments
Assignees

Comments

@eyalroz
Copy link
Owner

eyalroz commented Feb 27, 2019

We're getting lots and lots of warnings building dbgen with MSVC:

\src\bcd2.c(84): warning C4244: 'return': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
\src\bcd2.c(107): warning C4244: 'return': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
\src\bcd2.c(129): warning C4244: '=': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
\src\bcd2.c(130): warning C4244: '+=': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
\src\bcd2.c(134): warning C4244: '+=': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
\src\bcd2.c(144): warning C4244: 'return': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
\src\bcd2.c(165): warning C4244: 'return': conversion from '__int64' to 'int', possible loss of data [\dbgen.vcxproj]
  bm_utils.c
\src\bm_utils.c(111): warning C4273: 'getenv': inconsistent dll linkage [\dbgen.vcxproj]
  c:\program files (x86)\windows kits\10\include\10.0.17134.0\ucrt\stdlib.h(1191): note: see previous definition of 'getenv'
  build.c
\src\build.c(252): warning C4244: 'function': conversion from 'double' to 'long', possible loss of data [\dbgen.vcxproj]
  driver.c
\src\driver.c(842): warning C4244: '*=': conversion from 'double' to 'long', possible loss of data [\dbgen.vcxproj]
\src\driver.c(1157): warning C4244: '=': conversion from 'double' to 'long', possible loss of data [\dbgen.vcxproj]
  load_stub.c
  permute.c
  print.c
\src\print.c(764): warning C4244: '+=': conversion from '__int64' to 'unsigned long', possible loss of data [\dbgen.vcxproj]
  rnd.c
  speed_seed.c
\src\speed_seed.c(123): warning C4244: '=': conversion from '__int64' to 'RND', possible loss of data [\dbgen.vcxproj]
\src\speed_seed.c(135): warning C4244: 'function': conversion from '__int64' to 'RND', possible loss of data [\dbgen.vcxproj]
  text.c
  Generating Code...

and about the same errors when building qgen.

In this issue, we'll consider just the type conersion, not the DLL linkage warning. These might be due to the fact that we kept an int type for BCD arithmetic function return types. Let's change those and see what happens.

@valco1994 : FYI.

eyalroz pushed a commit that referenced this issue Feb 27, 2019
* Changed bcd arithmetic functions' return type from `int` to `DSS_HUGE`.
@eyalroz eyalroz self-assigned this Feb 27, 2019
eyalroz pushed a commit that referenced this issue Feb 27, 2019
* Changed bcd arithmetic functions' return type from `int` to `DSS_HUGE`.
* `VRF_HUGE` now uses `DSS_HUGE` instead of `long` for type casts.
* Switched the `Modulus` and `Multiplier` constants in `speed_seed.c` to have smaller types which fit their values, rather than DSS_HUGE.
eyalroz pushed a commit that referenced this issue Feb 27, 2019
* Changed bcd arithmetic functions' return type from `int` to `DSS_HUGE`.
* `VRF_HUGE` now uses `DSS_HUGE` instead of `long` for type casts.
* Switched the `Modulus` and `Multiplier` constants in `speed_seed.c` to have smaller types which fit their values, rather than DSS_HUGE.
eyalroz pushed a commit that referenced this issue Feb 27, 2019
* Changed the `bcd_` arithmetic functions' return type from `int` to `DSS_HUGE`.
* `VRF_HUGE` now uses `DSS_HUGE` instead of `long` for type casts.
* Switched the `Modulus` and `Multiplier` constants in `speed_seed.c` to have smaller types which fit their values, rather than DSS_HUGE.
eyalroz pushed a commit that referenced this issue Feb 27, 2019
* Changed the `bcd_` arithmetic functions' return type from `int` to `DSS_HUGE`.
* `VRF_HUGE` now uses `DSS_HUGE` instead of `long` for type casts.
* Switched the `Modulus` and `Multiplier` constants in `speed_seed.c` to have smaller types which fit their values, rather than DSS_HUGE.
* Additional changes of type to `DSS_HUGE` where it seems that's needed.
eyalroz pushed a commit that referenced this issue Feb 28, 2019
* Changed the `bcd_` arithmetic functions' return type from `int` to `DSS_HUGE`.
* `VRF_HUGE` now uses `DSS_HUGE` instead of `long` for type casts.
* Switched the `Modulus` and `Multiplier` constants in `speed_seed.c` to have smaller types which fit their values, rather than DSS_HUGE.
* Additional changes of type to `DSS_HUGE` where it seems that's needed.
@valco1994
Copy link
Collaborator

Ok, I will look at it too

valco1994 added a commit to valco1994/ssb-dbgen that referenced this issue Feb 28, 2019
@valco1994
Copy link
Collaborator

It's looking depressing. I have tried to fix part of warnings and get even more as result.

@eyalroz
Copy link
Owner Author

eyalroz commented Feb 28, 2019

Wait, don't do it yet. This is affected by changes regarding other issues (e.g. SUPPORT_64BITS).

@valco1994
Copy link
Collaborator

@eyalroz, hello! I try to write to you via e-mail, but Gmail notifies, that address not found, so I write here.

Maybe, it worths to merge changes from additional_fixes to master? There are a lot of nice fixes and improvements. And fix of warnings on MSVC required by this issue (#65) can be done later in a separate PR.

@eyalroz
Copy link
Owner Author

eyalroz commented Mar 20, 2019

If you triend [email protected] - that's gone. I'm at [email protected] .

eyalroz pushed a commit that referenced this issue Mar 21, 2019
* Changed the `bcd_` arithmetic functions' return type from `int` to `DSS_HUGE`.
* `VRF_HUGE` now uses `DSS_HUGE` instead of `long` for type casts.
* Switched the `Modulus` and `Multiplier` constants in `speed_seed.c` to have smaller types which fit their values, rather than DSS_HUGE.
* Additional changes of type to `DSS_HUGE` where it seems that's needed.
eyalroz pushed a commit that referenced this issue Mar 31, 2019
* The overall number of records uses the `DSS_HUGE` type; no longer assuming each update segment's row count can fit into an `int`
* The seed adjustment functions (`sd_order`, `sd_line` etc.) take a skip count which is a record index; it could well be a `DSS_HUGE`, seeing how the `row_cnt` global is a `DSS_HUGE`.
eyalroz pushed a commit that referenced this issue Mar 31, 2019
* The overall number of records uses the `DSS_HUGE` type; no longer assuming each update segment's row count can fit into an `int`
* The seed adjustment functions (`sd_order`, `sd_line` etc.) take a skip count which is a record index; but it is also a `long`, since the seeds are long. Now, we could start changing the seeds into `DSS_HUGE`s, but for now - let's just explicitly cast `DSS_HUGE` record count inputs to these function into `long`s.
eyalroz pushed a commit that referenced this issue Mar 31, 2019
* The overall number of records uses the `DSS_HUGE` type; no longer assuming each update segment's row count can fit into an `int`
* The seed adjustment functions (`sd_order`, `sd_line` etc.) take a skip count which is a record index; but it is also a `long`, since the seeds are long. Now, we could start changing the seeds into `DSS_HUGE`s, but for now - let's just explicitly cast `DSS_HUGE` record count inputs to these function into `long`s.
* Propagate use of `DSS_HUGE` in `pr_drange()`
@valco1994
Copy link
Collaborator

Useful information related to this issue can be found in a discussion of issue #69.
The most important points are:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants