Add ALWAYS and NEVER macros (from SQLite)

.github/workflows/test-fuzzer.yml

@@ -24,7 +24,7 @@ jobs:
         run: |
           mkdir build
           cd build
-          cmake -DCMAKE_BUILD_TYPE=Release ..
+          cmake -DCMAKE_BUILD_TYPE=Debug ..
 
       - name: Build
         run: |
@@ -57,7 +57,7 @@ jobs:
         run: |
           mkdir build
           cd build
-          cmake -DCMAKE_BUILD_TYPE=Release -DENABLE_LIBFUZZER=ON ..
+          cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_LIBFUZZER=ON ..
 
       - name: Build
         run: |

.github/workflows/test-linux.yml

@@ -8,11 +8,15 @@ on:
 
 jobs:
   gcc-tests:
-    name: Test Compile gcc
+    name: Test Compile gcc ${{ matrix.build_type }}
     runs-on: ubuntu-latest
     env:
       CC: gcc
 
+    strategy:
+      matrix:
+        build_type: ["Debug", "Release"]
+
     steps:
       - uses: actions/checkout@v2.4.0
 
@@ -22,7 +26,7 @@ jobs:
           sudo apt-get install doxygen graphviz clang-format-9
 
       - name: Configure build
-        run: cmake -Bbuild -DWARNINGS_AS_ERRORS=ON .
+        run: cmake -Bbuild -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DWARNINGS_AS_ERRORS=ON .
 
       - name: Formatting check
         working-directory: build
@@ -56,12 +60,12 @@ jobs:
         run: |
           mkdir build/examples
           cd build/examples
-          cmake ../../examples
+          cmake -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ../../examples
           make
           make test
 
   clang-tests:
-    name: Test Compile clang ${{ matrix.compile_opt }}
+    name: Test Compile clang ${{ matrix.build_type }} ${{ matrix.compile_opt }}
     runs-on: ubuntu-latest
     env:
       CC: clang
@@ -71,6 +75,7 @@ jobs:
         # See Clang docs for more information on the sanitizers:
         # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
         compile_opt: ["", "-fsanitize=undefined,float-divide-by-zero -fno-sanitize-recover=undefined,float-divide-by-zero", "-fsanitize=memory -fno-sanitize-recover=memory", "-fsanitize=address -fno-sanitize-recover=address"]
+        build_type: ["Debug", "Release"]
 
     steps:
       - uses: actions/checkout@v2.4.0
@@ -81,7 +86,7 @@ jobs:
           sudo apt-get install doxygen graphviz clang-format-9
 
       - name: Configure build
-        run: cmake -Bbuild -DWARNINGS_AS_ERRORS=ON -DCMAKE_C_FLAGS="${{ matrix.compile_opt }}" .
+        run: cmake -Bbuild -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DWARNINGS_AS_ERRORS=ON -DCMAKE_C_FLAGS="${{ matrix.compile_opt }}" .
 
       - name: Formatting check
         working-directory: build
@@ -115,7 +120,7 @@ jobs:
         run: |
           mkdir build/examples
           cd build/examples
-          cmake -DCMAKE_C_FLAGS="${{ matrix.compile_opt }}" ../../examples
+          cmake -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DCMAKE_C_FLAGS="${{ matrix.compile_opt }}" ../../examples
           make
           make test
 
@@ -159,7 +164,7 @@ jobs:
           sudo apt-get install lcov
 
       - name: Configure build
-        run: cmake -DCMAKE_BUILD_TYPE=Debug -DWARNINGS_AS_ERRORS=ON -DH3_PREFIX=testprefix_ .
+        run: cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_COVERAGE=ON -DWARNINGS_AS_ERRORS=ON -DH3_PREFIX=testprefix_ .
 
       - name: Build
         run: make

.github/workflows/test-macos.yml

@@ -8,14 +8,18 @@ on:
 
 jobs:
   tests:
-    name: Test Compile ${{ matrix.compiler }}
+    name: Test Compile ${{ matrix.build_type }}
     runs-on: macos-latest
 
+    strategy:
+      matrix:
+        build_type: ["Debug", "Release"]
+
     steps:
       - uses: actions/checkout@v2.4.0
 
       - name: Configure build
-        run: cmake -Bbuild -DWARNINGS_AS_ERRORS=ON .
+        run: cmake -Bbuild -DCMAKE_BUILD_TYPE=${{ matrix.build_type}} -DWARNINGS_AS_ERRORS=ON .
 
       - name: Build
         working-directory: build
@@ -37,6 +41,6 @@ jobs:
         run: |
           mkdir build/examples
           cd build/examples
-          cmake ../../examples
+          cmake -DCMAKE_BUILD_TYPE=${{ matrix.build_type}} ../../examples
           make
           make test

.github/workflows/test-windows.yml

@@ -26,7 +26,7 @@ jobs:
 
       - name: Configure build
         shell: cmd
-        run: cmake -Bbuild -A ${{ matrix.arch }} -DWARNINGS_AS_ERRORS=ON -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTING=ON .
+        run: cmake -Bbuild -A ${{ matrix.arch }} -DCMAKE_BUILD_TYPE=${{ matrix.build_type}} -DWARNINGS_AS_ERRORS=ON -DBUILD_SHARED_LIBS=OFF -DBUILD_TESTING=ON .
 
       - name: Build
         working-directory: build

CHANGELOG.md

@@ -10,6 +10,9 @@ The public API of this library consists of the functions declared in file
 - Fixed possible signed integer overflow in `h3NeighborRotations` (#707)
 - Fixed possible signed integer overflow in `localIjToCell` (#706)
 
+### Changed
+- `assert` on defensive code blocks that are not already covered. (#720)
+
 ## [4.0.1] - 2022-09-15
 ### Fixed
 - Changing an internal `float` to `double` improves the precision of geographic coordinate output (#652)

CMakeLists.txt

@@ -45,7 +45,7 @@ else()
     set(ENABLE_REQUIRES_ALL_SYMBOLS OFF)
 endif()
 
-option(ENABLE_COVERAGE "Enable compiling tests with coverage." ON)
+option(ENABLE_COVERAGE "Enable compiling tests with coverage." OFF)
 option(BUILD_BENCHMARKS "Build benchmarking applications." ON)
 option(BUILD_FUZZERS "Build fuzzer applications (for use with afl)." ON)
 option(BUILD_FILTERS "Build filter applications." ON)
@@ -120,6 +120,8 @@ if(H3_IS_ROOT_PROJECT)
 endif()
 
 set(LIB_SOURCE_FILES
+    src/h3lib/include/h3Assert.h
+    src/h3lib/include/alloc.h
     src/h3lib/include/bbox.h
     src/h3lib/include/polygon.h
     src/h3lib/include/polygonAlgos.h
@@ -138,6 +140,7 @@ set(LIB_SOURCE_FILES
     src/h3lib/include/constants.h
     src/h3lib/include/coordijk.h
     src/h3lib/include/algos.h
+    src/h3lib/lib/h3Assert.c
     src/h3lib/lib/algos.c
     src/h3lib/lib/coordijk.c
     src/h3lib/lib/bbox.c
@@ -288,6 +291,9 @@ function(add_h3_library name h3_alloc_prefix_override)
         set_target_properties(${name} PROPERTIES SOVERSION ${H3_SOVERSION})
         target_compile_definitions(${name} PRIVATE BUILD_SHARED_LIBS=1)
     endif()
+    if(ENABLE_COVERAGE)
+        target_compile_definitions(${name} PRIVATE H3_COVERAGE_TEST=1)
+    endif()
 
     target_compile_definitions(${name} PUBLIC H3_PREFIX=${H3_PREFIX})
     target_compile_definitions(${name} PRIVATE BUILDING_H3=1)

src/h3lib/include/alloc.h

@@ -24,7 +24,7 @@
 #ifndef ALLOC_H
 #define ALLOC_H
 
-#include "h3api.h" // for TJOIN
+#include "h3api.h"  // for TJOIN
 
 #ifdef H3_ALLOC_PREFIX
 #define H3_MEMORY(name) TJOIN(H3_ALLOC_PREFIX, name)
@@ -33,10 +33,10 @@
 extern "C" {
 #endif
 
-void* H3_MEMORY(malloc)(size_t size);
-void* H3_MEMORY(calloc)(size_t num, size_t size);
-void* H3_MEMORY(realloc)(void* ptr, size_t size);
-void H3_MEMORY(free)(void* ptr);
+void *H3_MEMORY(malloc)(size_t size);
+void *H3_MEMORY(calloc)(size_t num, size_t size);
+void *H3_MEMORY(realloc)(void *ptr, size_t size);
+void H3_MEMORY(free)(void *ptr);
 
 #ifdef __cplusplus
 }

src/h3lib/include/h3Assert.h

@@ -0,0 +1,125 @@
+/*
+ * Copyright 2022 Uber Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+/** @file h3Assert.h
+ * @brief   Support code for unit testing and assertions
+ *
+ * This file defines macros needed for defensive programming in the H3 core
+ * library. H3 strives to have complete code and branch coverage, but this is
+ * not feasible if some branches cannot be reached because they are defensive -
+ * that is, we do not know of a test case that would exercise the branch but we
+ * do have an opinion of how to recover from such an error. These defensive
+ * branches are excluded from coverage.
+ *
+ * In other testing, such as unit tests or fuzzer testing, they trigger
+ * assertions if the conditions fail.
+ *
+ * Adapted from https://www.sqlite.org/testing.html and
+ * https://www.sqlite.org/assert.html
+ *
+ * Used under the terms of the SQLite3 project, reproduced below:
+ * The author disclaims copyright to this source code.  In place of
+ * a legal notice, here is a blessing:
+ *
+ *    May you do good and not evil.
+ *    May you find forgiveness for yourself and forgive others.
+ *    May you share freely, never taking more than you give.
+ */
+
+#ifndef H3ASSERT_H
+#define H3ASSERT_H
+
+#include <assert.h>
+
+/*
+** The testcase() macro is used to aid in coverage testing.  When
+** doing coverage testing, the condition inside the argument to
+** testcase() must be evaluated both true and false in order to
+** get full branch coverage.  The testcase() macro is inserted
+** to help ensure adequate test coverage in places where simple
+** condition/decision coverage is inadequate.  For example, testcase()
+** can be used to make sure boundary values are tested.  For
+** bitmask tests, testcase() can be used to make sure each bit
+** is significant and used at least once.  On switch statements
+** where multiple cases go to the same block of code, testcase()
+** can insure that all cases are evaluated.
+*/
+#if defined(H3_COVERAGE_TEST) || defined(H3_DEBUG)
+extern unsigned int h3CoverageCounter;
+#define testcase(X)                              \
+    if (X) {                                     \
+        h3CoverageCounter += (unsigned)__LINE__; \
+    }
+#else
+#define testcase(X)
+#endif
+
+/*
+** Disable ALWAYS() and NEVER() (make them pass-throughs) for coverage
+** and mutation testing
+*/
+#if defined(H3_COVERAGE_TEST)
+#define H3_OMIT_AUXILIARY_SAFETY_CHECKS 1
+#endif
+
+/*
+** The TESTONLY macro is used to enclose variable declarations or
+** other bits of code that are needed to support the arguments
+** within testcase() and assert() macros.
+*/
+#if !defined(NDEBUG) || defined(H3_COVERAGE_TEST)
+#define TESTONLY(X) X
+#else
+#define TESTONLY(X)
+#endif
+
+/*
+** The DEFENSEONLY macro is used to enclose variable declarations or
+** other bits of code that are needed to support the arguments
+** within ALWAYS() or NEVER() macros.
+*/
+#if !defined(H3_OMIT_AUXILIARY_SAFETY_CHECKS)
+#define DEFENSEONLY(X) X
+#else
+#define DEFENSEONLY(X)
+#endif
+
+/*
+** The ALWAYS and NEVER macros surround boolean expressions which
+** are intended to always be true or false, respectively.  Such
+** expressions could be omitted from the code completely.  But they
+** are included in a few cases in order to enhance the resilience
+** of the H3 library to unexpected behavior - to make the code "self-healing"
+** or "ductile" rather than being "brittle" and crashing at the first
+** hint of unplanned behavior.
+**
+** In other words, ALWAYS and NEVER are added for defensive code.
+**
+** When doing coverage testing ALWAYS and NEVER are hard-coded to
+** be true and false so that the unreachable code they specify will
+** not be counted as untested code.
+*/
+#if defined(H3_OMIT_AUXILIARY_SAFETY_CHECKS)
+#define ALWAYS(X) (1)
+#define NEVER(X) (0)
+#elif !defined(NDEBUG)
+#define ALWAYS(X) ((X) ? 1 : (assert(0), 0))
+#define NEVER(X) ((X) ? (assert(0), 1) : 0)
+#else
+#define ALWAYS(X) (X)
+#define NEVER(X) (X)
+#endif
+
+#endif