# housekeeping (improve unreachable code annotations and avoid compiler warnings)

adts: use SAFE_FREE
config: add CONFIG_FINAL; use it in
compression,vfs_optimizer,scriptablecomplex
lib: add discussion of unreachable code avoidance; revise UNREACHABLE
and add NODEFAULT
ogl_tex, tex_dds: remove some unnecessary UNREACHABLE
sysdep: move compiler-specific implementation of UNREACHABLE here.
use NODEFAULT in snd, wdbg, wposix, wsdl

This was SVN commit r3739.

source/lib/adts.h

@@ -117,11 +117,7 @@ public:
 	{
 		// note: users might call clear() right before the dtor runs,
 		// so safely handling calling this twice.
-		if(tbl)
-		{
-			free(tbl);
-			tbl = 0;
-		}
+		SAFE_FREE(tbl);
 		num_entries = 0;
 		// rationale: must not set to 0 because expand_tbl only doubles the size.
 		// don't keep the previous size because it may have become huge and

source/lib/config.h

@@ -46,6 +46,12 @@
 # define CONFIG_PARANOIA 0
 #endif
 
+// finale release; disables some safety checks.
+#ifndef CONFIG_FINAL
+# define CONFIG_FINAL 0
+#endif
+
+
 // enable trace output for low-level code - various functions will
 // debug_printf when they are entered/exited. note that the appropriate
 // TRACEn tags must be debug_filter_add-ed for this to have any effect.

source/lib/lib.h

@@ -152,21 +152,75 @@ STMT(\
 // .. wrapped around the parameter name, e.g. void f(int UNUSED(x))
 #define UNUSED(param)
 
-// indicates a piece of code cannot be reached (e.g. because all
-// control paths before it end up returning). this is mostly for
-// human benefit, but it may also help optimization and generates
-// warnings if reached in paranoia builds.
-#if MSC_VERSION
-// .. note: we only enable this in paranoia builds because it
-//    causes "unreachable code" warnings (exactly what we want to avoid).
-# if CONFIG_PARANOIA
-#  define UNREACHABLE debug_warn("hit supposedly unreachable code");
+
+/*
+"unreachable code" helpers
+
+unreachable lines of code are often the source or symptom of subtle bugs.
+they are flagged by compiler warnings; however, the opposite problem -
+erroneously reaching certain spots (e.g. due to missing return statement)
+is worse and not detected automatically.
+
+to defend against this, the programmer can annotate their code to
+indicate to humans that a particular spot should never be reached.
+however, that isn't much help; better is a sentinel that raises an
+error if if it is actually reached. hence, the UNREACHABLE macro.
+
+ironically, if the code guarded by UNREACHABLE works as it should,
+compilers may flag the macro's code as unreachable. this would
+distract from genuine warnings, which is unacceptable.
+
+even worse, compilers differ in their code checking: GCC only complains if
+non-void functions end without returning a value (i.e. missing return
+statement), while VC checks if lines are unreachable (e.g. if they are
+preceded by a return on all paths).
+
+our implementation of UNREACHABLE solves this dilemna as follows:
+- on GCC: call abort(); since it has the noreturn attributes, the
+  "non-void" warning disappears.
+- on VC: avoid generating any code. we allow the compiler to assume the
+  spot is actually unreachable, which incidentally helps optimization.
+  if reached after all, a crash usually results. in that case, compile with
+  CONFIG_PARANOIA, which will cause an error message to be displayed.
+
+this approach still allows for the possiblity of automated
+checking, but does not cause any compiler warnings.
+*/
+
+// 1) final build: optimize assuming this location cannot be reached.
+//    may crash if that turns out to be untrue, but removes checking overhead.
+#if CONFIG_FINAL
+# define UNREACHABLE SYS_UNREACHABLE
+// 2) normal build:
 #else
-#  define UNREACHABLE __assume(0)
-# endif
+//    a) normal implementation: includes "abort", which is declared with
+//       noreturn attribute and therefore avoids GCC's "execution reaches
+//       end of non-void function" warning.
+# if !MSC_VERSION || CONFIG_PARANOIA
+#  define UNREACHABLE\
+	STMT(\
+		debug_warn("hit supposedly unreachable code");\
+		abort();\
+	)
+//    b) VC only: don't generate any code; squelch the warning and optimize.
 # else
-# define UNREACHABLE
+#  define UNREACHABLE SYS_UNREACHABLE
 # endif
+#endif
+
+/*
+convenient specialization of UNREACHABLE for switch statements whose
+default can never be reached. example usage:
+int x;
+switch(x % 2)
+{
+	case 0: break;
+	case 1: break;
+	NODEFAULT;
+}
+*/
+#define NODEFAULT default: UNREACHABLE
+
 
 #define ARRAY_SIZE(name) (sizeof(name) / sizeof(name[0]))
 

source/lib/res/file/compression.cpp

@@ -290,7 +290,7 @@ public:
 			// we therefore enable this only in final builds; during
 			// development, 1.5% bigger archives are definitely worth much
 			// faster build time.
-#ifdef FINAL
+#if CONFIG_FINAL
 			const int level      = Z_BEST_COMPRESSION;
 #else
 			const int level      = Z_BEST_SPEED;

source/lib/res/file/vfs_mount.cpp

@@ -857,7 +857,6 @@ LibError x_validate(const XFile* xf)
 	default:
 		return ERR_INVALID_MOUNT_TYPE;
 	}
-	UNREACHABLE;
 }
 
 
@@ -956,7 +955,6 @@ LibError x_io_validate(const XIo* xio)
 	default:
 		return ERR_INVALID_MOUNT_TYPE;
 	}
-	UNREACHABLE;
 }
 
 

source/lib/res/file/vfs_optimizer.cpp

@@ -484,20 +484,20 @@ public:
 // for each loose or archived file encountered during mounting: add to a
 // std::set; if there are more than *_THRESHOLD non-archived files, rebuild.
 // this ends up costing 50ms for 5000 files, so disable it in final release.
-#ifndef FINAL
-# define AB_COUNT_LOOSE_FILES 1
-#else
+#if CONFIG_FINAL
 # define AB_COUNT_LOOSE_FILES 0
+#else
+# define AB_COUNT_LOOSE_FILES 1
 #endif
 // rebuild if the archive is much older than most recent VFS timestamp.
 // this makes sense during development: the archive will periodically be
 // rebuilt with the newest trace. however, it would be annoying in the
 // final release, where users will frequently mod things, which should not
 // end up rebuilding the main archive.
-#ifndef FINAL
-# define AB_COMPARE_MTIME 1
-#else
+#if CONFIG_FINAL
 # define AB_COMPARE_MTIME 0
+#else
+# define AB_COMPARE_MTIME 1
 #endif
 
 #if AB_COUNT_LOOSE_FILES

source/lib/res/graphics/ogl_shader.cpp

@@ -426,7 +426,6 @@ LibError ogl_program_use(Handle h)
 	{
 		pglUseProgramObjectARB(0);
 		WARN_RETURN(ERR_INVALID_HANDLE);
-		UNREACHABLE;
 	}
 
 	pglUseProgramObjectARB(p->id);

source/lib/res/graphics/ogl_tex.cpp

@@ -45,7 +45,6 @@ static bool filter_valid(GLint filter)
 	default:
 		return false;
 	}
-	UNREACHABLE;
 }
 
 
@@ -62,7 +61,6 @@ static bool wrap_valid(GLint wrap)
 	default:
 		return false;
 	}
-	UNREACHABLE;
 }
 
 
@@ -78,7 +76,6 @@ static bool filter_uses_mipmaps(GLint filter)
 	default:
 		return false;
 	}
-	UNREACHABLE;
 }
 
 
@@ -94,7 +91,6 @@ static bool fmt_is_s3tc(GLenum fmt)
 	default:
 		return false;
 	}
-	UNREACHABLE;
 }
 
 

source/lib/res/graphics/tex_dds.cpp

@@ -380,7 +380,6 @@ static bool is_valid_dxt(uint dxt)
 	default:
 		return false;
 	}
-	UNREACHABLE;
 }
 
 

source/lib/res/sound/snd.cpp

@@ -1259,8 +1259,7 @@ static FadeRet fade(FadeInfo& fi, double cur_time, float& out_val)
 		factor = 0.0f;
 		break;
 
-	default:
-		UNREACHABLE;
+	NODEFAULT;
 	}
 
 	out_val = fi.initial_val + factor*(fi.final_val - fi.initial_val);

source/lib/sysdep/sysdep.h

@@ -282,7 +282,17 @@ extern size_t sys_max_sector_size();
 #define DIR_SEP '/'
 #endif
 
-
+// tell the compiler that the code at/following this macro invocation is
+// unreachable. this can improve optimization and avoid warnings.
+//
+// this macro should not generate any fallback code; it is merely the
+// compiler-specific backend for lib.h's UNREACHABLE.
+// #define it to nothing if the compiler doesn't support such a hint.
+#if MSC_VERSION
+# define SYS_UNREACHABLE __assume(0)
+#else
+# define SYS_UNREACHABLE
+#endif
 
 
 #ifdef __cplusplus

source/lib/sysdep/win/wdbg.cpp

@@ -291,7 +291,7 @@ have_reg:
 	case 1:	context->Dr1 = addr; break;
 	case 2:	context->Dr2 = addr; break;
 	case 3:	context->Dr3 = addr; break;
-	default: UNREACHABLE;
+	NODEFAULT;
 	}
 
 	// choose breakpoint settings:

source/lib/sysdep/win/wposix.cpp

@@ -581,8 +581,7 @@ static DWORD win32_prot(int prot)
 		return PAGE_EXECUTE_READWRITE;
 	case PROT_READ|PROT_WRITE|PROT_EXEC:
 		return PAGE_EXECUTE_READWRITE;
-	default:
-		UNREACHABLE;
+	NODEFAULT;
 	}
 }
 

source/lib/sysdep/win/wsdl.cpp

@@ -842,8 +842,7 @@ static LRESULT OnMouseButton(HWND UNUSED(hWnd), UINT uMsg, int x, int y, UINT UN
 		button = SDL_BUTTON_MIDDLE;
 		state = SDL_RELEASED;
 		break;
-	default:
-		UNREACHABLE;
+	NODEFAULT;
 	}
 
 	// mouse capture

source/scripting/ScriptableComplex.h

@@ -964,7 +964,9 @@ public:
 	// m_Properties[PropertyName] (avoids mem leak).
 	void DeletePreviouslyAssignedProperty( CStrW PropertyName )
 	{
-#ifndef FINAL
+#if !CONFIG_FINAL
+		UNUSED2(PropertyName);
+#else
 		PropertyTable::iterator it;
 		it = m_Properties.find( PropertyName );
 		if( it != m_Properties.end() )
@@ -972,8 +974,6 @@ public:
 			debug_warn("BUG: CJSComplexProperty added but already existed!");
 			jscomplexproperty_suballoc_free(it->second);
 		}
-#else
-		UNUSED2(PropertyName);
 #endif
 	}