fix: Incorrect operator precedence in conditional assembly expressions

 (#259)

clients/vscode-hlasmplugin/CHANGELOG.md

@@ -20,6 +20,7 @@
 - Empty arrays now behave similarly to other subscripted variables in macro tracer
 - Forward structured macro parameters correctly
 - Empty operands ignored by the SETx instructions
+- Incorrect operator precedence in conditional assembly expressions
 
 ## [1.1.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.0.0...1.1.0) (2022-03-29)
 

language_server/test/virtual_file_provider_test.cpp

@@ -46,7 +46,7 @@ TEST(virtual_file_provider, predicate)
 
 TEST(virtual_file_provider, file_missing)
 {
-    ws_mngr_mock ws_mngr;
+    NiceMock<ws_mngr_mock> ws_mngr;
     json_sink_mock sink;
 
     virtual_file_provider vfp(ws_mngr, sink);
@@ -75,7 +75,7 @@ TEST(virtual_file_provider, file_missing)
 
 TEST(virtual_file_provider, file_present)
 {
-    ws_mngr_mock ws_mngr;
+    NiceMock<ws_mngr_mock> ws_mngr;
     json_sink_mock sink;
 
     virtual_file_provider vfp(ws_mngr, sink);

parser_library/src/expressions/conditional_assembly/ca_expr_policy.cpp

@@ -26,13 +26,6 @@ bool ca_character_policy::is_unary(ca_expr_ops op)
         || op == ca_expr_ops::UPPER;
 }
 
-bool ca_arithmetic_policy::multiple_words(ca_expr_ops) { return false; }
-bool ca_binary_policy::multiple_words(ca_expr_ops op)
-{
-    return op == ca_expr_ops::AND || op == ca_expr_ops::OR || op == ca_expr_ops::XOR;
-}
-bool ca_character_policy::multiple_words(ca_expr_ops) { return false; }
-
 bool ca_arithmetic_policy::is_binary(ca_expr_ops op)
 {
     return op == ca_expr_ops::SLA || op == ca_expr_ops::SLL || op == ca_expr_ops::SRA || op == ca_expr_ops::SRL
@@ -44,8 +37,7 @@ bool ca_binary_policy::is_binary(ca_expr_ops op)
 {
     return op == ca_expr_ops::EQ || op == ca_expr_ops::NE || op == ca_expr_ops::LE || op == ca_expr_ops::LT
         || op == ca_expr_ops::GE || op == ca_expr_ops::GT || op == ca_expr_ops::AND || op == ca_expr_ops::OR
-        || op == ca_expr_ops::XOR || op == ca_expr_ops::AND_NOT || op == ca_expr_ops::OR_NOT
-        || op == ca_expr_ops::XOR_NOT;
+        || op == ca_expr_ops::XOR;
 }
 
 bool ca_character_policy::is_binary(ca_expr_ops) { return false; }
@@ -156,13 +148,10 @@ int ca_binary_policy::get_priority(ca_expr_ops op)
         case ca_expr_ops::NOT:
             return 1;
         case ca_expr_ops::AND:
-        case ca_expr_ops::AND_NOT:
             return 2;
         case ca_expr_ops::OR:
-        case ca_expr_ops::OR_NOT:
             return 3;
         case ca_expr_ops::XOR:
-        case ca_expr_ops::XOR_NOT:
             return 4;
         default:
             return 0;
@@ -289,9 +278,6 @@ ca_expr_ops get_expr_operator(const std::string& op)
     S2O(SRL);
     S2O(FIND);
     S2O(INDEX);
-    S2O(AND_NOT);
-    S2O(OR_NOT);
-    S2O(XOR_NOT);
     S2O(EQ);
     S2O(NE);
     S2O(LE);
@@ -312,8 +298,54 @@ ca_expr_ops get_expr_operator(const std::string& op)
 }
 
 ca_expr_ops ca_arithmetic_policy::get_operator(const std::string& symbol) { return get_expr_operator(symbol); }
+std::variant<std::monostate, invalid_by_policy, ca_expr_op> ca_arithmetic_policy::get_operator_properties(
+    const std::string& symbol)
+{
+    auto o = get_operator(symbol);
+    if (o == ca_expr_ops::UNKNOWN)
+        return std::monostate();
+    bool unary = is_unary(o);
+    bool binary = is_binary(o);
+    if (!unary && !binary)
+        return invalid_by_policy();
+    return ca_expr_op { o, get_priority(o), binary, false, unary };
+}
+
 ca_expr_ops ca_binary_policy::get_operator(const std::string& symbol) { return get_expr_operator(symbol); }
+std::variant<std::monostate, invalid_by_policy, ca_expr_op> ca_binary_policy::get_operator_properties(
+    const std::string& symbol)
+{
+    auto o = get_operator(symbol);
+    if (o == ca_expr_ops::UNKNOWN)
+        return std::monostate();
+    bool unary = is_unary(o);
+    bool binary = is_binary(o);
+    if (!unary && !binary)
+        return invalid_by_policy();
+    return ca_expr_op {
+        o,
+        get_priority(o),
+        binary,
+        o == ca_expr_ops::EQ || o == ca_expr_ops::NE || o == ca_expr_ops::LE || o == ca_expr_ops::LT
+            || o == ca_expr_ops::GE || o == ca_expr_ops::GT,
+        unary,
+    };
+}
+
 ca_expr_ops ca_character_policy::get_operator(const std::string& symbol) { return get_expr_operator(symbol); }
+std::variant<std::monostate, invalid_by_policy, ca_expr_op> ca_character_policy::get_operator_properties(
+    const std::string& symbol)
+{
+    auto o = get_operator(symbol);
+    if (o == ca_expr_ops::UNKNOWN)
+        return std::monostate();
+    bool unary = is_unary(o);
+    bool binary = is_binary(o);
+    if (!unary && !binary)
+        return invalid_by_policy();
+    return ca_expr_op { o, get_priority(o), binary, false, unary };
+}
+
 
 ca_expr_funcs ca_arithmetic_policy::get_function(const std::string& symbol)
 {

parser_library/src/expressions/conditional_assembly/ca_expr_policy.h

@@ -15,6 +15,8 @@
 #ifndef HLASMPLUGIN_PARSERLIBRARY_CA_EXPR_POLICY_H
 #define HLASMPLUGIN_PARSERLIBRARY_CA_EXPR_POLICY_H
 
+#include <variant>
+
 #include "context/common_types.h"
 #include "context/id_storage.h"
 
@@ -34,9 +36,6 @@ enum class ca_expr_ops
     INDEX,
 
     // logical
-    AND_NOT,
-    OR_NOT,
-    XOR_NOT,
     EQ,
     NE,
     LE,
@@ -60,6 +59,18 @@ enum class ca_expr_ops
     UNKNOWN
 };
 
+struct ca_expr_op
+{
+    ca_expr_ops op;
+    int priority;
+    bool binary;
+    bool requires_terms;
+    bool right_assoc;
+};
+
+struct invalid_by_policy
+{};
+
 enum class ca_expr_funcs
 {
     // arithmetic
@@ -120,9 +131,6 @@ class ca_arithmetic_policy
     // is binary arithmetic operation
     static bool is_binary(ca_expr_ops op);
 
-    // true if op can consist of multiple words (eg AND NOT)
-    static bool multiple_words(ca_expr_ops op);
-
     // get priority relative to rest of arithmetic operators
     static int get_priority(ca_expr_ops op);
 
@@ -135,6 +143,9 @@ class ca_arithmetic_policy
     // transforms string operator to enum
     static ca_expr_ops get_operator(const std::string& symbol);
 
+    static std::variant<std::monostate, invalid_by_policy, ca_expr_op> get_operator_properties(
+        const std::string& symbol);
+
     // transforms string function to enum
     static ca_expr_funcs get_function(const std::string& symbol);
 
@@ -157,9 +168,6 @@ class ca_binary_policy
     // is binary binary operation
     static bool is_binary(ca_expr_ops op);
 
-    // true if op can consist of multiple words (eg AND NOT)
-    static bool multiple_words(ca_expr_ops op);
-
     // get priority relative to rest of binary operators
     static int get_priority(ca_expr_ops op);
 
@@ -172,6 +180,9 @@ class ca_binary_policy
     // transforms string operator to enum
     static ca_expr_ops get_operator(const std::string& symbol);
 
+    static std::variant<std::monostate, invalid_by_policy, ca_expr_op> get_operator_properties(
+        const std::string& symbol);
+
     // transforms string function to enum
     static ca_expr_funcs get_function(const std::string& symbol);
 
@@ -194,9 +205,6 @@ class ca_character_policy
     // is binary character operation
     static bool is_binary(ca_expr_ops op);
 
-    // true if op can consist of multiple words (eg AND NOT)
-    static bool multiple_words(ca_expr_ops op);
-
     // get priority relative to rest of character operators
     static int get_priority(ca_expr_ops op);
 
@@ -209,6 +217,9 @@ class ca_character_policy
     // transforms string operator to enum
     static ca_expr_ops get_operator(const std::string& symbol);
 
+    static std::variant<std::monostate, invalid_by_policy, ca_expr_op> get_operator_properties(
+        const std::string& symbol);
+
     // transforms string function to enum
     static ca_expr_funcs get_function(const std::string& symbol);
 

parser_library/src/expressions/conditional_assembly/ca_operator_binary.cpp

@@ -205,12 +205,6 @@ context::SET_t ca_function_binary_operator::operation(
                 return lhs.access_b() || rhs.access_b();
             case ca_expr_ops::XOR:
                 return lhs.access_b() != rhs.access_b();
-            case ca_expr_ops::AND_NOT:
-                return lhs.access_b() && !rhs.access_b();
-            case ca_expr_ops::OR_NOT:
-                return lhs.access_b() || !rhs.access_b();
-            case ca_expr_ops::XOR_NOT:
-                return lhs.access_b() != !rhs.access_b();
             default:
                 break;
         }