XPath: Remove the use of fallthrough switch cases

We were previously relying on non-standard comment detection that is
supported by gcc/clang to avoid warnings about implicit fallthrough.

This can be solved using attributes but using them requires a lot of
compiler-specific detection logic because not all versions of gcc/clang
support them.

We don't *really* need to rely on fallthrough here - the type conversion
block can be located *after* the AST type switch instead, which means
that any AST type that has type ambiguity can fall back to that in the
future.

Fixes #331.

1 files changed, 72 insertions, 67 deletions

diff --git a/src/pugixml.cpp b/src/pugixml.cpp
index 9b6c1b6..cdaf9ca 100644
--- a/src/pugixml.cpp
+++ b/src/pugixml.cpp
@@ -10415,36 +10415,37 @@ PUGI__NS_BEGIN
 				if (_rettype == xpath_type_boolean)
 					return _data.variable->get_boolean();
 
-				// variable needs to be converted to the correct type, this is handled by the default statement below
+				// variable needs to be converted to the correct type, this is handled by the fallthrough block below
+				break;
 			}
 
-			// fallthrough
 			default:
-			{
-				switch (_rettype)
-				{
-				case xpath_type_number:
-					return convert_number_to_boolean(eval_number(c, stack));
+				;
+			}
 
-				case xpath_type_string:
-				{
-					xpath_allocator_capture cr(stack.result);
+			// none of the ast types that return the value directly matched, we need to perform type conversion
+			switch (_rettype)
+			{
+			case xpath_type_number:
+				return convert_number_to_boolean(eval_number(c, stack));
 
-					return !eval_string(c, stack).empty();
-				}
+			case xpath_type_string:
+			{
+				xpath_allocator_capture cr(stack.result);
 
-				case xpath_type_node_set:
-				{
-					xpath_allocator_capture cr(stack.result);
+				return !eval_string(c, stack).empty();
+			}
 
-					return !eval_node_set(c, stack, nodeset_eval_any).empty();
-				}
+			case xpath_type_node_set:
+			{
+				xpath_allocator_capture cr(stack.result);
 
-				default:
-					assert(false && "Wrong expression for return type boolean"); // unreachable
-					return false;
-				}
+				return !eval_node_set(c, stack, nodeset_eval_any).empty();
 			}
+
+			default:
+				assert(false && "Wrong expression for return type boolean"); // unreachable
+				return false;
 			}
 		}
 
@@ -10552,37 +10553,37 @@ PUGI__NS_BEGIN
 				if (_rettype == xpath_type_number)
 					return _data.variable->get_number();
 
-				// variable needs to be converted to the correct type, this is handled by the default statement below
+				// variable needs to be converted to the correct type, this is handled by the fallthrough block below
+				break;
 			}
 
-			// fallthrough
 			default:
-			{
-				switch (_rettype)
-				{
-				case xpath_type_boolean:
-					return eval_boolean(c, stack) ? 1 : 0;
-
-				case xpath_type_string:
-				{
-					xpath_allocator_capture cr(stack.result);
+				;
+			}
 
-					return convert_string_to_number(eval_string(c, stack).c_str());
-				}
+			// none of the ast types that return the value directly matched, we need to perform type conversion
+			switch (_rettype)
+			{
+			case xpath_type_boolean:
+				return eval_boolean(c, stack) ? 1 : 0;
 
-				case xpath_type_node_set:
-				{
-					xpath_allocator_capture cr(stack.result);
+			case xpath_type_string:
+			{
+				xpath_allocator_capture cr(stack.result);
 
-					return convert_string_to_number(eval_string(c, stack).c_str());
-				}
+				return convert_string_to_number(eval_string(c, stack).c_str());
+			}
 
-				default:
-					assert(false && "Wrong expression for return type number"); // unreachable
-					return 0;
-				}
+			case xpath_type_node_set:
+			{
+				xpath_allocator_capture cr(stack.result);
 
+				return convert_string_to_number(eval_string(c, stack).c_str());
 			}
+
+			default:
+				assert(false && "Wrong expression for return type number"); // unreachable
+				return 0;
 			}
 		}
 
@@ -10838,35 +10839,36 @@ PUGI__NS_BEGIN
 				if (_rettype == xpath_type_string)
 					return xpath_string::from_const(_data.variable->get_string());
 
-				// variable needs to be converted to the correct type, this is handled by the default statement below
+				// variable needs to be converted to the correct type, this is handled by the fallthrough block below
+				break;
 			}
 
-			// fallthrough
 			default:
-			{
-				switch (_rettype)
-				{
-				case xpath_type_boolean:
-					return xpath_string::from_const(eval_boolean(c, stack) ? PUGIXML_TEXT("true") : PUGIXML_TEXT("false"));
+				;
+			}
 
-				case xpath_type_number:
-					return convert_number_to_string(eval_number(c, stack), stack.result);
+			// none of the ast types that return the value directly matched, we need to perform type conversion
+			switch (_rettype)
+			{
+			case xpath_type_boolean:
+				return xpath_string::from_const(eval_boolean(c, stack) ? PUGIXML_TEXT("true") : PUGIXML_TEXT("false"));
 
-				case xpath_type_node_set:
-				{
-					xpath_allocator_capture cr(stack.temp);
+			case xpath_type_number:
+				return convert_number_to_string(eval_number(c, stack), stack.result);
 
-					xpath_stack swapped_stack = {stack.temp, stack.result};
+			case xpath_type_node_set:
+			{
+				xpath_allocator_capture cr(stack.temp);
 
-					xpath_node_set_raw ns = eval_node_set(c, swapped_stack, nodeset_eval_first);
-					return ns.empty() ? xpath_string() : string_value(ns.first(), stack.result);
-				}
+				xpath_stack swapped_stack = {stack.temp, stack.result};
 
-				default:
-					assert(false && "Wrong expression for return type string"); // unreachable
-					return xpath_string();
-				}
+				xpath_node_set_raw ns = eval_node_set(c, swapped_stack, nodeset_eval_first);
+				return ns.empty() ? xpath_string() : string_value(ns.first(), stack.result);
 			}
+
+			default:
+				assert(false && "Wrong expression for return type string"); // unreachable
+				return xpath_string();
 			}
 		}
 
@@ -10989,14 +10991,17 @@ PUGI__NS_BEGIN
 					return ns;
 				}
 
-				// variable needs to be converted to the correct type, this is handled by the default statement below
+				// variable needs to be converted to the correct type, this is handled by the fallthrough block below
+				break;
 			}
 
-			// fallthrough
 			default:
-				assert(false && "Wrong expression for return type node set"); // unreachable
-				return xpath_node_set_raw();
+				;
 			}
+
+			// none of the ast types that return the value directly matched, but conversions to node set are invalid
+			assert(false && "Wrong expression for return type node set"); // unreachable
+			return xpath_node_set_raw();
 		}
 
 		void optimize(xpath_allocator* alloc)