Conversation
|
coverage: 99.162%. remained the same — ajfriend:aj/header-only-coordijk-v2 into uber:master |
| if(ENABLE_COVERAGE) | ||
| target_compile_definitions(${name} PRIVATE H3_COVERAGE_TEST=1) | ||
| endif() |
There was a problem hiding this comment.
This is needed because some of the test executables call coordijk.h code directly?
There was a problem hiding this comment.
Yeah, it's called from these:
- testCoordIjkInternal.c
- fuzzerInternalCoordIjk.c
| // edge-crossing vertices as needed | ||
| g->numVerts = 0; | ||
| FaceIJK lastFijk; | ||
| FaceIJK lastFijk = {0}; |
There was a problem hiding this comment.
For my info, what's the difference here?
There was a problem hiding this comment.
The previous build was just complaining about potentially using it uninitialized: https://github.com/uber/h3/actions/runs/24574942537/job/71857687680#step:6:82
But it's a false positive from moving this to the header; lastFijk is always initialized before use. This just silences the warning.
nrabinowitz
left a comment
There was a problem hiding this comment.
LGTM - I didn't go over the code in detail, assuming that this was pure lift/shift rather than editing
Same as #1148, but cleaner git history.
Benchmarks comparing this PR to the current master, run with https://github.com/ajfriend/h3-index-bench: