Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions docs/developers_guide/input_availability.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,25 @@ handled.

```text
expression := or-expression
or-expression := and-expression ("or" and-expression)*
and-expression := primary (("and" | ",") primary)*
or-expression := and-expression (" or " and-expression)*

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check now aligns parse_availability() with set_availability(), but the documented grammar is still broader than the accepted canonical language. For example, the grammar admits

(basis_type==pw)

and

basis_type==pw and calculation==scf or esolver_type==sdft

while the new tests explicitly reject both. Likewise, the grammar permits "pw" as a quoted value, but the serializer normalizes it to pw.

Referring to “the spelling produced by the AST serializer” does not precisely document the accepted syntax. Could you update the grammar to describe the serializer’s canonical language explicitly, including the required parentheses around compound subexpressions and the canonical quoting rules for values? The current strict parser behavior should be preserved, in my opinion.

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 '"'
```

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
`relax_method=="cg 2"`. Two or more alternatives use `in [...]`, while
Expand Down
15 changes: 9 additions & 6 deletions source/source_io/module_parameter/availability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,8 @@ class AvailabilityParser
AvailabilityExpr result = parse_primary();
std::vector<AvailabilityExpr> 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)
Expand Down Expand Up @@ -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
11 changes: 5 additions & 6 deletions source/source_io/module_parameter/availability.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 1 addition & 9 deletions source/source_io/module_parameter/input_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include <functional>
#include <map>
#include <sstream>
#include <stdexcept>
#include <string>
#include <vector>

Expand Down Expand Up @@ -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
Expand Down
20 changes: 19 additions & 1 deletion source/source_io/module_parameter/test/availability_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading