From c8081f6c7f4a0aa9cf96406c69c5d5dd1e7a4034 Mon Sep 17 00:00:00 2001 From: Stardust0831 <169599847+Stardust0831@users.noreply.github.com> Date: Sat, 15 Aug 2026 01:24:02 +0800 Subject: [PATCH 1/3] fix(input): align canonical availability grammar --- docs/developers_guide/input_availability.md | 16 +++++++++++---- .../module_parameter/availability.cpp | 15 ++++++++------ .../source_io/module_parameter/availability.h | 11 +++++----- .../source_io/module_parameter/input_item.h | 10 +--------- .../test/availability_test.cpp | 20 ++++++++++++++++++- 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/docs/developers_guide/input_availability.md b/docs/developers_guide/input_availability.md index 8bad5f2c97..1e0946cae4 100644 --- a/docs/developers_guide/input_availability.md +++ b/docs/developers_guide/input_availability.md @@ -26,16 +26,24 @@ handled. ```text expression := or-expression -or-expression := and-expression ("or" and-expression)* -and-expression := primary (("and" | ",") primary)* +or-expression := and-expression (" or " and-expression)* +and-expression := primary (" and " primary)* primary := condition | "(" expression ")" 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 '"' ``` +The spaces shown inside grammar literals are required: comparison operators +have no surrounding spaces, `and`, `or`, `in`, and `contains` use one space on +each side, and list commas are followed by one space. 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`, `mode in [a,b]`, and +`basis_type==pw,calculation==scf` are rejected. + `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 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..b30e9cf674 100644 --- a/source/source_io/module_parameter/test/availability_test.cpp +++ b/source/source_io/module_parameter/test/availability_test.cpp @@ -109,7 +109,25 @@ 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 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"); From 4e1d09398b184e2f4d748abdd0c2b61d996f984b Mon Sep 17 00:00:00 2001 From: Stardust0831 <169599847+Stardust0831@users.noreply.github.com> Date: Sat, 15 Aug 2026 14:13:19 +0800 Subject: [PATCH 2/3] docs(input): clarify canonical availability spacing --- docs/developers_guide/input_availability.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/developers_guide/input_availability.md b/docs/developers_guide/input_availability.md index 1e0946cae4..d6cbe8941a 100644 --- a/docs/developers_guide/input_availability.md +++ b/docs/developers_guide/input_availability.md @@ -36,13 +36,14 @@ comparison := "==" | "!=" | ">" | ">=" | "<" | "<=" value := token | '"' quoted-value '"' ``` -The spaces shown inside grammar literals are required: comparison operators -have no surrounding spaces, `and`, `or`, `in`, and `contains` use one space on -each side, and list commas are followed by one space. 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`, `mode in [a,b]`, and -`basis_type==pw,calculation==scf` are rejected. +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. `and` binds more tightly than `or`. `==` compares one complete value; double quotes delimit a complete value containing whitespace, such as From bd8bf87b322a1f2138a0b1cf33ec0593312fd089 Mon Sep 17 00:00:00 2001 From: Stardust0831 <169599847+Stardust0831@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:20:54 +0800 Subject: [PATCH 3/3] docs(input): define canonical availability language --- docs/developers_guide/input_availability.md | 34 +++++++++++++------ .../test/availability_test.cpp | 1 + 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/docs/developers_guide/input_availability.md b/docs/developers_guide/input_availability.md index d6cbe8941a..5435082bf0 100644 --- a/docs/developers_guide/input_availability.md +++ b/docs/developers_guide/input_availability.md @@ -25,17 +25,26 @@ 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 comparison := "==" | "!=" | ">" | ">=" | "<" | "<=" -value := token | '"' quoted-value '"' +value := bare-value | '"' quote-required-value '"' ``` +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, @@ -45,12 +54,15 @@ 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. -`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. +`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/test/availability_test.cpp b/source/source_io/module_parameter/test/availability_test.cpp index b30e9cf674..ea44f9a250 100644 --- a/source/source_io/module_parameter/test/availability_test.cpp +++ b/source/source_io/module_parameter/test/availability_test.cpp @@ -117,6 +117,7 @@ TEST(AvailabilityParser, NonCanonicalSpellingIsRejected) "(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", };