diff --git a/docs/developers_guide/input_availability.md b/docs/developers_guide/input_availability.md index 8bad5f2c97..5435082bf0 100644 --- a/docs/developers_guide/input_availability.md +++ b/docs/developers_guide/input_availability.md @@ -25,23 +25,44 @@ handled. ## Grammar and meaning ```text -expression := or-expression -or-expression := and-expression ("or" and-expression)* -and-expression := primary (("and" | ",") primary)* -primary := condition | "(" expression ")" +expression := condition | compound +compound := operand " and " operand (" and " operand)* + | operand " or " operand (" or " operand)* +operand := condition | "(" compound ")" condition := parameter comparison value - | parameter "in" "[" value "," value ("," value)* "]" - | parameter "contains" value + | parameter " in [" value ", " value (", " value)* "]" + | parameter " contains " value comparison := "==" | "!=" | ">" | ">=" | "<" | "<=" -value := token | '"' quoted-value '"' +value := bare-value | '"' quote-required-value '"' ``` -`and` binds more tightly than `or`. `==` compares one complete value; double -quotes delimit a complete value containing whitespace, such as -`relax_method=="cg 2"`. Two or more alternatives use `in [...]`, while -`parameter contains value` tests whether a vector contains one element, such as -`td_ttype contains 0`. `/` is an ordinary value character, not another spelling -of membership. Ordered comparisons require a numeric scalar. +The grammar describes the canonical form of a non-empty expression. A compound +expression contains only one kind of Boolean operator. When a compound +expression is used as an operand of another compound expression, it must be +parenthesized, even when operator precedence would make the grouping +unambiguous. A single condition must not be parenthesized. For example, +`(basis_type==pw and calculation==scf) or esolver_type==sdft` is canonical, +while `basis_type==pw and calculation==scf or esolver_type==sdft` and +`(basis_type==pw)` are not. + +Canonical formatting is required: comparison operators have no surrounding +spaces; `and`, `or`, `in`, and `contains` have one space on each side; and list +items are separated by a comma followed by one space. For example, +`mode in [a, b]` is valid, while `mode in [a,b]` is rejected. Leading or +trailing whitespace is not allowed, and a comma is not an alternative spelling +of `and`. The parser accepts only the spelling produced by the AST serializer, +so forms such as `basis_type == pw` and +`basis_type==pw,calculation==scf` are also rejected. + +`bare-value` is a non-empty value containing no whitespace or any of +`()[],=<>!`. A value containing at least one of those characters must use the +non-empty double-quoted form, and a value that does not require quotes must not +use them. Thus `basis_type==pw` and `relax_method=="cg 2"` are canonical, while +`basis_type=="pw"` is not. `==` compares one complete value. Two or more +alternatives use `in [...]`, while `parameter contains value` tests whether a +vector contains one element, such as `td_ttype contains 0`. `/` is an ordinary +value character, not another spelling of membership. Ordered comparisons +require a numeric scalar. A path that references a parameter must imply that parameter's availability. Every `and` operand is required, while satisfying either branch of an `or` is diff --git a/source/source_io/module_parameter/availability.cpp b/source/source_io/module_parameter/availability.cpp index 611fa0526e..5db4d4e51a 100644 --- a/source/source_io/module_parameter/availability.cpp +++ b/source/source_io/module_parameter/availability.cpp @@ -248,12 +248,8 @@ class AvailabilityParser AvailabilityExpr result = parse_primary(); std::vector children; children.push_back(result); - while (true) + while (consume_keyword("and")) { - if (!consume_keyword("and") && !consume_char(',')) - { - break; - } children.push_back(parse_primary()); } if (children.size() == 1) @@ -342,7 +338,14 @@ std::string AvailabilityExpr::to_string() const AvailabilityExpr parse_availability(const std::string& raw) { - return AvailabilityParser(raw).parse(); + AvailabilityExpr expression = AvailabilityParser(raw).parse(); + const std::string canonical = expression.to_string(); + if (raw != canonical) + { + throw std::invalid_argument("Non-canonical availability expression '" + raw + + "'; expected '" + canonical + "'"); + } + return expression; } } // namespace ModuleIO diff --git a/source/source_io/module_parameter/availability.h b/source/source_io/module_parameter/availability.h index 0eabc619f9..489546cb48 100644 --- a/source/source_io/module_parameter/availability.h +++ b/source/source_io/module_parameter/availability.h @@ -34,13 +34,12 @@ struct AvailabilityExpr std::string to_string() const; }; -/// Parse an availability string into its boolean-expression tree. +/// Parse a canonical availability string into its boolean-expression tree. /// -/// Accepts the canonical grammar (`param==value`, `param in [a, b]`, the -/// comparison operators ==, !=, >, >=, <, <=, `and`/`or`/`,` combinators and -/// `(...)` grouping). An empty string yields an empty (always-available) -/// expression. Non-empty input that is not consumed by this grammar throws -/// std::invalid_argument. +/// The input must exactly match the AST serialization (`param==value`, +/// `param in [a, b]`, comparison operators, `and`/`or` combinators and `(...)` +/// grouping). An empty string yields an empty (always-available) expression. +/// Invalid or non-canonical input throws std::invalid_argument. AvailabilityExpr parse_availability(const std::string& raw); } // namespace ModuleIO diff --git a/source/source_io/module_parameter/input_item.h b/source/source_io/module_parameter/input_item.h index b77a656bd4..0e78f4e32a 100644 --- a/source/source_io/module_parameter/input_item.h +++ b/source/source_io/module_parameter/input_item.h @@ -3,7 +3,6 @@ #include #include #include -#include #include #include @@ -54,14 +53,7 @@ class Input_Item /// means that the item is always available. void set_availability(const std::string& value) { - const AvailabilityExpr parsed = parse_availability(value); - const std::string canonical = parsed.to_string(); - if (value != canonical) - { - throw std::invalid_argument("Non-canonical availability expression '" + value - + "'; expected '" + canonical + "'"); - } - availability_expr_ = parsed; + availability_expr_ = parse_availability(value); } std::string get_availability() const diff --git a/source/source_io/module_parameter/test/availability_test.cpp b/source/source_io/module_parameter/test/availability_test.cpp index 8987c90bdd..ea44f9a250 100644 --- a/source/source_io/module_parameter/test/availability_test.cpp +++ b/source/source_io/module_parameter/test/availability_test.cpp @@ -109,7 +109,26 @@ TEST(AvailabilityParser, InvalidExpressionsAreRejected) } } -TEST(AvailabilityParser, SetterRequiresCanonicalInputAndPreservesStateOnFailure) +TEST(AvailabilityParser, NonCanonicalSpellingIsRejected) +{ + const char* noncanonical_expressions[] = { + "basis_type == pw", + "basis_type==pw ", + "(basis_type==pw)", + "basis_type==pw,calculation==scf", + "vdw_method in [d2,d3_0]", + "basis_type==\"pw\"", + "basis_type==pw and(calculation==scf)", + "basis_type==pw and calculation==scf or esolver_type==sdft", + }; + for (const char* expression : noncanonical_expressions) + { + EXPECT_THROW(parse_availability(expression), std::invalid_argument) + << expression; + } +} + +TEST(AvailabilityParser, SetterPreservesStateOnParseFailure) { Input_Item item("example"); item.set_availability("basis_type==pw");