Enable build warnings/errors on Windows #831

kou · 2024-04-25T02:57:17Z

src/h3lib/lib/coodijk.c:

(73,10): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
(74,10): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
(133,25): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
(137,25): warning C4244: '=': conversion from 'double' to 'int', possible loss of data
(236,13): warning C4307: '+': signed integral constant overflow

src/h3lib/lib/localij.c:

(633,14): warning C4244: 'initializing': conversion from 'double' to 'int', possible loss of data
(634,14): warning C4244: 'initializing': conversion from 'double' to 'int', possible loss of data
(635,14): warning C4244: 'initializing': conversion from 'double' to 'int', possible loss of data

src/apps/testapps/testH3Memory.c:

(84,21): warning C4098: 'test_prefix_free': 'void' function returning a value

src/apps/testapps/testMathExtensionsInternal.c:

(81,9): warning C4307: '-': signed integral constant overflow
(82,9): warning C4307: '-': signed integral constant overflow
(83,9): warning C4307: '-': signed integral constant overflow
(84,9): warning C4307: '-': signed integral constant overflow
(85,9): warning C4307: '-': signed integral constant overflow
(86,9): warning C4307: '-': signed integral constant overflow
(87,9): warning C4307: '-': signed integral constant overflow
(88,9): warning C4307: '-': signed integral constant overflow
(89,9): warning C4307: '-': signed integral constant overflow
(90,9): warning C4307: '-': signed integral constant overflow
(91,9): warning C4307: '-': signed integral constant overflow
(92,9): warning C4307: '-': signed integral constant overflow
(93,9): warning C4307: '-': signed integral constant overflow
(94,9): warning C4307: '-': signed integral constant overflow
(95,9): warning C4307: '-': signed integral constant overflow
(96,9): warning C4307: '-': signed integral constant overflow
(97,9): warning C4307: '-': signed integral constant overflow
(98,9): warning C4307: '-': signed integral constant overflow
(99,9): warning C4307: '-': signed integral constant overflow
(100,9): warning C4307: '-': signed integral constant overflow
(101,9): warning C4307: '-': signed integral constant overflow
(102,9): warning C4307: '-': signed integral constant overflow
(103,9): warning C4307: '-': signed integral constant overflow
(104,9): warning C4307: '-': signed integral constant overflow
(105,9): warning C4307: '-': signed integral constant overflow
(106,9): warning C4307: '-': signed integral constant overflow

src/apps/testapps/testCellToLocalIj.c:

(204,44): warning C4146: unary minus operator applied to unsigned type, result still unsigned

src/h3lib/lib/coodijk.c: (73,10): warning C4244: '=': conversion from 'double' to 'int', possible loss of data (74,10): warning C4244: '=': conversion from 'double' to 'int', possible loss of data (133,25): warning C4244: '=': conversion from 'double' to 'int', possible loss of data (137,25): warning C4244: '=': conversion from 'double' to 'int', possible loss of data (236,13): warning C4307: '+': signed integral constant overflow src/h3lib/lib/localij.c: (633,14): warning C4244: 'initializing': conversion from 'double' to 'int', possible loss of data (634,14): warning C4244: 'initializing': conversion from 'double' to 'int', possible loss of data (635,14): warning C4244: 'initializing': conversion from 'double' to 'int', possible loss of data src/apps/testapps/testH3Memory.c: (84,21): warning C4098: 'test_prefix_free': 'void' function returning a value src/apps/testapps/testMathExtensionsInternal.c: (81,9): warning C4307: '-': signed integral constant overflow (82,9): warning C4307: '-': signed integral constant overflow (83,9): warning C4307: '-': signed integral constant overflow (84,9): warning C4307: '-': signed integral constant overflow (85,9): warning C4307: '-': signed integral constant overflow (86,9): warning C4307: '-': signed integral constant overflow (87,9): warning C4307: '-': signed integral constant overflow (88,9): warning C4307: '-': signed integral constant overflow (89,9): warning C4307: '-': signed integral constant overflow (90,9): warning C4307: '-': signed integral constant overflow (91,9): warning C4307: '-': signed integral constant overflow (92,9): warning C4307: '-': signed integral constant overflow (93,9): warning C4307: '-': signed integral constant overflow (94,9): warning C4307: '-': signed integral constant overflow (95,9): warning C4307: '-': signed integral constant overflow (96,9): warning C4307: '-': signed integral constant overflow (97,9): warning C4307: '-': signed integral constant overflow (98,9): warning C4307: '-': signed integral constant overflow (99,9): warning C4307: '-': signed integral constant overflow (100,9): warning C4307: '-': signed integral constant overflow (101,9): warning C4307: '-': signed integral constant overflow (102,9): warning C4307: '-': signed integral constant overflow (103,9): warning C4307: '-': signed integral constant overflow (104,9): warning C4307: '-': signed integral constant overflow (105,9): warning C4307: '-': signed integral constant overflow (106,9): warning C4307: '-': signed integral constant overflow src/apps/testapps/testCellToLocalIj.c: (204,44): warning C4146: unary minus operator applied to unsigned type, result still unsigned

coveralls · 2024-04-25T17:17:48Z

coverage: 98.824% (-0.003%) from 98.827%
when pulling 5a6cea4 on kou:windows-warning
into afaa41a on uber:master.

isaacbrodsky · 2024-04-25T18:11:35Z

Thanks for adding warnings-on-Windows! I'm not entirely certain why coveralls is reporting coverage fell, it might be because of changing the macro to an inline function.

CMakeLists.txt

dfellis · 2024-04-25T19:40:49Z

src/h3lib/include/mathExtensions.h

+static inline bool ADD_INT32S_OVERFLOWS(int32_t a, int32_t b) {
+    if (a > 0) {
+        return INT32_MAX - a < b;
+    } else {
+        return INT32_MIN - a > b;
+    }
+}


What is the purpose of these two changes? Do they actually affect the compiler warnings in some way?

It's for suppressing the following warnings (that are listed in the description):

src/h3lib/lib/coodijk.c:

(236,13): warning C4307: '+': signed integral constant overflow

src/apps/testapps/testMathExtensionsInternal.c:

(81,9): warning C4307: '-': signed integral constant overflow (82,9): warning C4307: '-': signed integral constant overflow (83,9): warning C4307: '-': signed integral constant overflow (84,9): warning C4307: '-': signed integral constant overflow (85,9): warning C4307: '-': signed integral constant overflow (86,9): warning C4307: '-': signed integral constant overflow (87,9): warning C4307: '-': signed integral constant overflow (88,9): warning C4307: '-': signed integral constant overflow (89,9): warning C4307: '-': signed integral constant overflow (90,9): warning C4307: '-': signed integral constant overflow (91,9): warning C4307: '-': signed integral constant overflow (92,9): warning C4307: '-': signed integral constant overflow (93,9): warning C4307: '-': signed integral constant overflow (94,9): warning C4307: '-': signed integral constant overflow (95,9): warning C4307: '-': signed integral constant overflow (96,9): warning C4307: '-': signed integral constant overflow (97,9): warning C4307: '-': signed integral constant overflow (98,9): warning C4307: '-': signed integral constant overflow (99,9): warning C4307: '-': signed integral constant overflow (100,9): warning C4307: '-': signed integral constant overflow (101,9): warning C4307: '-': signed integral constant overflow (102,9): warning C4307: '-': signed integral constant overflow (103,9): warning C4307: '-': signed integral constant overflow (104,9): warning C4307: '-': signed integral constant overflow (105,9): warning C4307: '-': signed integral constant overflow (106,9): warning C4307: '-': signed integral constant overflow

It seems that MSVC can't process ... ? ... : ... correctly for this warning yet.

Co-authored-by: David Ellis <isv.damocles@gmail.com>

Adds `ENABLE_WARNINGS` to the build docs

giordano · 2025-03-06T12:02:42Z

CMakeLists.txt

 set(H3_LINK_FLAGS "")
+option(ENABLE_WARNINGS "Enables compiler warnings" ON)
+if(ENABLE_WARNINGS)
+    if(WIN32)


This is wrong, /W2 is a MSVC flag and you should check MSVC instead. MSVC isn't the only Windows compiler available.

Thanks for spotting! Would you mind filing an issue for this?

Oh, sorry! I've opened an issue for this: #980

I'll open a PR too.

isaacbrodsky approved these changes Apr 25, 2024

View reviewed changes

dfellis reviewed Apr 25, 2024

View reviewed changes

Add ENABLE_WARNINGS option

5a6cea4

Co-authored-by: David Ellis <isv.damocles@gmail.com>

dfellis approved these changes Apr 27, 2024

View reviewed changes

isaacbrodsky merged commit c6c6850 into uber:master Apr 28, 2024

isaacbrodsky added a commit that referenced this pull request Apr 28, 2024

Update compilation-options.md for #831

6c59dc4

Adds `ENABLE_WARNINGS` to the build docs

isaacbrodsky added a commit that referenced this pull request Apr 28, 2024

Update compilation-options.md for #831 (#833)

fc72f22

Adds `ENABLE_WARNINGS` to the build docs

kou deleted the windows-warning branch April 29, 2024 02:47

giordano reviewed Mar 6, 2025

View reviewed changes

giordano mentioned this pull request Mar 6, 2025

[H3] Update to version 4.2.0 JuliaPackaging/Yggdrasil#10689

Merged

kou mentioned this pull request Mar 6, 2025

Windows: Wrong condition for ENABLE_WARNINGS #980

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enable build warnings/errors on Windows #831

Enable build warnings/errors on Windows #831

Uh oh!

kou commented Apr 25, 2024

Uh oh!

coveralls commented Apr 25, 2024 •

edited

Loading

Uh oh!

isaacbrodsky commented Apr 25, 2024

Uh oh!

Uh oh!

dfellis Apr 25, 2024

Uh oh!

kou Apr 25, 2024

Uh oh!

giordano Mar 6, 2025

Uh oh!

ajfriend Mar 6, 2025

Uh oh!

kou Mar 6, 2025

Uh oh!

kou Mar 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Enable build warnings/errors on Windows #831

Enable build warnings/errors on Windows #831

Uh oh!

Conversation

kou commented Apr 25, 2024

Uh oh!

coveralls commented Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isaacbrodsky commented Apr 25, 2024

Uh oh!

Uh oh!

dfellis Apr 25, 2024

Choose a reason for hiding this comment

Uh oh!

kou Apr 25, 2024

Choose a reason for hiding this comment

Uh oh!

giordano Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

ajfriend Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

kou Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

kou Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

coveralls commented Apr 25, 2024 •

edited

Loading