[flang-commits] [flang] [flang][OpenMP] Make parsing of trait properties more context-sensitive (PR #122900)

[Krzysztof Parzyszek via flang-commits]

================
@@ -158,75 +158,153 @@ static TypeDeclarationStmt makeIterSpecDecl(std::list<ObjectName> &&names) {
 static std::string nameToString(Name &&name) { return name.ToString(); }
 
 TYPE_PARSER(sourced(construct<OmpTraitPropertyName>( //
-    (space >> charLiteralConstantWithoutKind) ||
-    applyFunction(nameToString, Parser<Name>{}))))
+    construct<OmpTraitPropertyName>(space >> charLiteralConstantWithoutKind) ||
+    construct<OmpTraitPropertyName>(
+        applyFunction(nameToString, Parser<Name>{})))))
 
 TYPE_PARSER(sourced(construct<OmpTraitScore>( //
-    "SCORE" >> parenthesized(scalarIntExpr))))
+    "SCORE"_id >> parenthesized(scalarIntExpr))))
 
-TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension::ExtensionValue>(
-    // Parse nested extension first.
-    construct<OmpTraitPropertyExtension::ExtensionValue>(
-        indirect(Parser<OmpTraitPropertyExtension>{})) ||
-    construct<OmpTraitPropertyExtension::ExtensionValue>(
-        Parser<OmpTraitPropertyName>{}) ||
-    construct<OmpTraitPropertyExtension::ExtensionValue>(scalarExpr))))
-
-TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension>( //
+TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension::Complex>(
     Parser<OmpTraitPropertyName>{},
     parenthesized(nonemptySeparated(
-        Parser<OmpTraitPropertyExtension::ExtensionValue>{}, ","_tok)))))
+        indirect(Parser<OmpTraitPropertyExtension>{}), ",")))))
 
-TYPE_PARSER(sourced(construct<OmpTraitProperty>(
-    // Try clause first, then extension before OmpTraitPropertyName.
-    construct<OmpTraitProperty>(indirect(Parser<OmpClause>{})) ||
-    construct<OmpTraitProperty>(Parser<OmpTraitPropertyExtension>{}) ||
-    construct<OmpTraitProperty>(Parser<OmpTraitPropertyName>{}) ||
-    construct<OmpTraitProperty>(scalarExpr))))
+TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension>(
+    construct<OmpTraitPropertyExtension>(
+        Parser<OmpTraitPropertyExtension::Complex>{}) ||
+    construct<OmpTraitPropertyExtension>(Parser<OmpTraitPropertyName>{}) ||
+    construct<OmpTraitPropertyExtension>(scalarExpr))))
 
 TYPE_PARSER(construct<OmpTraitSelectorName::Value>(
-    "ARCH" >> pure(OmpTraitSelectorName::Value::Arch) ||
-    "ATOMIC_DEFAULT_MEM_ORDER" >>
+    "ARCH"_id >> pure(OmpTraitSelectorName::Value::Arch) ||
+    "ATOMIC_DEFAULT_MEM_ORDER"_id >>
         pure(OmpTraitSelectorName::Value::Atomic_Default_Mem_Order) ||
-    "CONDITION" >> pure(OmpTraitSelectorName::Value::Condition) ||
-    "DEVICE_NUM" >> pure(OmpTraitSelectorName::Value::Device_Num) ||
-    "EXTENSION" >> pure(OmpTraitSelectorName::Value::Extension) ||
-    "ISA" >> pure(OmpTraitSelectorName::Value::Isa) ||
-    "KIND" >> pure(OmpTraitSelectorName::Value::Kind) ||
-    "REQUIRES" >> pure(OmpTraitSelectorName::Value::Requires) ||
-    "SIMD" >> pure(OmpTraitSelectorName::Value::Simd) ||
-    "UID" >> pure(OmpTraitSelectorName::Value::Uid) ||
-    "VENDOR" >> pure(OmpTraitSelectorName::Value::Vendor)))
+    "CONDITION"_id >> pure(OmpTraitSelectorName::Value::Condition) ||
+    "DEVICE_NUM"_id >> pure(OmpTraitSelectorName::Value::Device_Num) ||
+    "EXTENSION"_id >> pure(OmpTraitSelectorName::Value::Extension) ||
+    "ISA"_id >> pure(OmpTraitSelectorName::Value::Isa) ||
+    "KIND"_id >> pure(OmpTraitSelectorName::Value::Kind) ||
+    "REQUIRES"_id >> pure(OmpTraitSelectorName::Value::Requires) ||
+    "SIMD"_id >> pure(OmpTraitSelectorName::Value::Simd) ||
+    "UID"_id >> pure(OmpTraitSelectorName::Value::Uid) ||
+    "VENDOR"_id >> pure(OmpTraitSelectorName::Value::Vendor)))
----------------
kparzysz wrote:

By default the parser will recognize a given string when it's a prefix of another string.  For example `"ATOMIC" >> ...` will trigger for `ATOMIC_DEFAULT_MEM_ORDER`, causing a syntax error which is rather non-obvious (the parser then looks at `_DEFAULT_MEM_ORDER`).  Adding "_id" to the string ensures that it will only trigger when it's delimited.

I don't remember what exactly I ran into here, but to eliminate the risk of these kinds of issues, I added "_id" to the strings in these parsers.

Do you have concerns with this?

https://github.com/llvm/llvm-project/pull/122900