#14476 Align CAF_ASSERT with standard assert semantics

CAF_ASSERT was unconditionally active in every build configuration. Make it
follow the semantics of the standard assert(): active in Debug, compiled out in
optimized builds (NDEBUG).

Add the CMake option RESINSIGHT_ENABLE_ASSERTS_IN_RELEASE (default OFF) for
developers who want the asserts to stay active in an optimized build, which is
useful when reproducing a problem in RelWithDebInfo with a debugger attached.

When compiled out, the expression is kept inside an unevaluated sizeof rather
than discarded. It is not evaluated, so there is no run-time cost and no side
effects, but it is still type checked and any variable used only by the assert
still counts as referenced, avoiding a wave of unused-variable warnings.

CAF_ENABLE_ASSERTS is given a default in cafAssert.h so the header stays
self-contained and include order can never silently switch the asserts off.

Document in docs/agents/coding-style.md that CAF_ASSERT is the assert to use,
that CVF_ASSERT is legacy, and that asserts are for broken invariants rather
than for run-time conditions that need real error handling.

CMakeLists.txt

@@ -140,6 +140,25 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
   set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DNO_DEBUG")
 endif()
 
+# ##############################################################################
+# Asserts
+# ##############################################################################
+
+# CAF_ASSERT follows the semantics of the standard assert(): active in Debug,
+# compiled out in optimized builds (Release, RelWithDebInfo, MinSizeRel). Enable
+# this option to keep the asserts active in an optimized build as well, which is
+# useful when reproducing a problem in a RelWithDebInfo build with a debugger
+# attached.
+option(RESINSIGHT_ENABLE_ASSERTS_IN_RELEASE
+       "Keep CAF_ASSERT active in optimized builds (Release, RelWithDebInfo)"
+       OFF
+)
+mark_as_advanced(RESINSIGHT_ENABLE_ASSERTS_IN_RELEASE)
+if(RESINSIGHT_ENABLE_ASSERTS_IN_RELEASE)
+  message(STATUS "Enabling CAF_ASSERT in optimized builds")
+  add_definitions(-DCAF_ENABLE_ASSERTS=1)
+endif()
+
 # ##############################################################################
 # OpenMP
 # ##############################################################################

Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafAssert.h

@@ -4,12 +4,48 @@
 #include <cstdio>
 #include <cstdlib>
 
-#define CAF_ASSERT( expr )                                                                   \
-    do                                                                                       \
-    {                                                                                        \
-        if ( !( expr ) ) /* NOLINT */                                                        \
-        {                                                                                    \
-            std::printf( "%s : %i : CAF_ASSERT( %s ) failed\n", __FILE__, __LINE__, #expr ); \
-            std::abort();                                                                    \
-        }                                                                                    \
+// CAF_ASSERT documents an invariant that must hold if the program is correct. It is meant for
+// programming errors, not for run-time conditions that can legitimately occur (bad input files,
+// missing data, failed network calls) - those need real error handling.
+//
+// Following the semantics of the standard assert(), CAF_ASSERT is active in debug builds and
+// compiled out in optimized builds (NDEBUG). A failing assert prints file/line/expression and
+// calls std::abort(), which raises SIGABRT and is picked up by the crash handler installed in
+// RiaMain.cpp, so the failure is written to the log file and reported to OpenTelemetry with a
+// stack trace.
+//
+// Developers who want the asserts to stay active in an optimized build (typically RelWithDebInfo)
+// can configure with -DRESINSIGHT_ENABLE_ASSERTS_IN_RELEASE=ON, which defines CAF_ENABLE_ASSERTS=1
+// for the whole build.
+//
+// CAF_ENABLE_ASSERTS is given a default here so this header stays self-contained: include order can
+// never silently switch the asserts off. See OPM/ResInsight#14397 for the class of bug this avoids.
+#ifndef CAF_ENABLE_ASSERTS
+#ifdef NDEBUG
+#define CAF_ENABLE_ASSERTS 0
+#else
+#define CAF_ENABLE_ASSERTS 1
+#endif
+#endif
+
+#if CAF_ENABLE_ASSERTS == 1
+
+#define CAF_ASSERT( expr )                                                                            \
+    do                                                                                                \
+    {                                                                                                 \
+        if ( !( expr ) ) /* NOLINT */                                                                 \
+        {                                                                                             \
+            std::fprintf( stderr, "%s : %i : CAF_ASSERT( %s ) failed\n", __FILE__, __LINE__, #expr ); \
+            std::abort();                                                                             \
+        }                                                                                             \
     } while ( false )
+
+#else
+
+// The expression is kept inside an unevaluated sizeof rather than discarded outright. It is not
+// evaluated, so there is no run-time cost and no side effects, but it is still type checked (the
+// assert cannot bit-rot into something that no longer compiles) and any variable used only by the
+// assert still counts as referenced, which avoids a wave of unused-variable warnings.
+#define CAF_ASSERT( expr ) ( (void)sizeof( !( expr ) ) ) /* NOLINT */
+
+#endif

docs/agents/coding-style.md

@@ -112,6 +112,36 @@ RiaLogging::info( QString( "Exported %1 cells for well '%2'." ).arg( count ).arg
 RiaLogging::info( std::format( "Exported {} cells for well '{}'.", count, wellName ) );
 ```
 
+### Asserts
+
+Use `CAF_ASSERT` (`cafAssert.h`). Do not add new `CVF_ASSERT`, `CVF_ASSERT_MSG`, `CVF_FAIL_MSG` or `CVF_TIGHT_ASSERT` — those are legacy and are being migrated away from.
+
+An assert documents an invariant that holds whenever the program is correct. It is not error handling. Conditions that can legitimately occur at run time — a missing file, a malformed input deck, a failed network call, a user selecting nothing — must be handled with a return value, `std::optional`, `std::expected` or a logged error message, never with an assert.
+
+```cpp
+// Bad – a missing file is a run-time condition, not a broken invariant
+CAF_ASSERT( QFile::exists( fileName ) );
+
+// Good – handled and reported
+if ( !QFile::exists( fileName ) )
+{
+    RiaLogging::error( std::format( "File not found: {}", fileName ) );
+    return {};
+}
+
+// Good – a broken invariant, unreachable unless the code is wrong
+CAF_ASSERT( index >= 0 && index < m_values.size() );
+```
+
+`CAF_ASSERT` follows the semantics of the standard `assert()`:
+
+- **Debug builds**: active. A failure prints file, line and expression, then calls `std::abort()`.
+- **Optimized builds** (Release, RelWithDebInfo): compiled out. The expression is still type checked but never evaluated, so it must be free of side effects — never put work inside the assert that the surrounding code depends on.
+
+Configure with `-DRESINSIGHT_ENABLE_ASSERTS_IN_RELEASE=ON` to keep the asserts active in an optimized build, which is useful when reproducing a problem in RelWithDebInfo with a debugger attached.
+
+A failing assert aborts via `SIGABRT`, which the crash handler installed in `RiaMain.cpp` picks up: the failure is written to the log file and reported to OpenTelemetry with a stack trace.
+
 ### Lambda Functions
 
 Keep lambdas short and readable: