From 76c3914484f1ec260b927fa98b4c77a25b9047c0 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 19 Feb 2020 07:32:53 -0800 Subject: [PATCH] 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. --- src/pugixml.cpp | 169 +++++++++++++++++++++++++----------------------- 1 file changed, 87 insertions(+), 82 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); - - return !eval_string(c, stack).empty(); - } - - case xpath_type_node_set: - { - xpath_allocator_capture cr(stack.result); - - return !eval_node_set(c, stack, nodeset_eval_any).empty(); - } - - default: - assert(false && "Wrong expression for return type boolean"); // unreachable - return false; - } + ; } + + // 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)); + + case xpath_type_string: + { + xpath_allocator_capture cr(stack.result); + + return !eval_string(c, stack).empty(); + } + + case xpath_type_node_set: + { + xpath_allocator_capture cr(stack.result); + + 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()); - } - - 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; - } - + ; } + + // 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_string: + { + xpath_allocator_capture cr(stack.result); + + return convert_string_to_number(eval_string(c, stack).c_str()); + } + + 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); - - case xpath_type_node_set: - { - xpath_allocator_capture cr(stack.temp); - - xpath_stack swapped_stack = {stack.temp, stack.result}; - - 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(); - } + ; } + + // 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_number: + return convert_number_to_string(eval_number(c, stack), stack.result); + + case xpath_type_node_set: + { + xpath_allocator_capture cr(stack.temp); + + xpath_stack swapped_stack = {stack.temp, stack.result}; + + 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)