fix: Reaching internal ACTR limit now only generates warnings

clients/vscode-hlasmplugin/CHANGELOG.md

@@ -11,6 +11,7 @@
 - Fixed an issue preventing correct N' attribute evaluation of empty subscript arrays
 - Contents of subscript arrays are now visible during debugging without the need to expand them
 - Improve detection of HLASM files
+- Reaching ACTR limit now only generates warnings
 
 ## [1.1.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.0.0...1.1.0) (2022-03-29)
 

parser_library/src/compiler_options.h

@@ -30,6 +30,8 @@ struct asm_option
 
     static constexpr unsigned int sysopt_rent_default = 0;
     unsigned int sysopt_rent = sysopt_rent_default;
+
+    long long statement_count_limit = 10'000'000;
 };
 } // namespace hlasm_plugin::parser_library
 #endif

parser_library/src/context/code_scope.h

@@ -37,22 +37,17 @@ struct code_scope
     // gets macro to which this scope belong (nullptr if in open code)
     macro_invo_ptr this_macro;
     // the ACTR branch counter
-    A_t branch_counter;
+    A_t branch_counter = 4096;
     // number of changed branch counters
-    size_t branch_counter_change;
+    size_t branch_counter_change = 0;
 
     bool is_in_macro() const { return !!this_macro; }
 
     code_scope(macro_invo_ptr macro_invo, macro_def_ptr macro_def)
         : this_macro(std::move(macro_invo))
-        , branch_counter(4096)
-        , branch_counter_change(0)
         , this_macro_def_(std::move(macro_def))
     {}
-    code_scope()
-        : branch_counter(4096)
-        , branch_counter_change(0)
-    {}
+    code_scope() = default;
 
 private:
     macro_def_ptr this_macro_def_;

parser_library/src/context/common_types.cpp

@@ -14,6 +14,7 @@
 
 #include "common_types.h"
 
+#include <cassert>
 #include <cctype>
 
 namespace hlasm_plugin::parser_library::context {
@@ -79,4 +80,25 @@ const B_t& SET_t::access_b() const { return b_value; }
 
 const C_t& SET_t::access_c() const { return c_value; }
 
+bool SET_t::operator==(const SET_t& r) const noexcept
+{
+    if (type != r.type)
+        return false;
+
+    switch (type)
+    {
+        case SET_t_enum::A_TYPE:
+            return a_value == r.a_value;
+        case SET_t_enum::B_TYPE:
+            return b_value == r.b_value;
+        case SET_t_enum::C_TYPE:
+            return c_value == r.c_value;
+        case SET_t_enum::UNDEF_TYPE:
+            return true;
+        default:
+            assert(false);
+            return false;
+    }
+}
+
 } // namespace hlasm_plugin::parser_library::context

parser_library/src/context/common_types.h

@@ -115,6 +115,8 @@ struct SET_t
     const A_t& access_a() const;
     const B_t& access_b() const;
     const C_t& access_c() const;
+
+    bool operator==(const SET_t& r) const noexcept;
 };
 
 // just mock method for now, will be implemented later with respect to UTF/EBCDIC

parser_library/src/context/hlasm_context.cpp

@@ -320,6 +320,7 @@ hlasm_context::hlasm_context(std::string file_name, asm_option asm_options, std:
     , instruction_map_(init_instruction_map(*ids_))
     , m_usings(std::make_unique<using_collection>())
     , m_active_usings(1, m_usings->remove_all())
+    , m_statements_remaining(asm_options_.statement_count_limit)
     , ord_ctx(*ids_, *this)
 {
     add_global_system_vars(scope_stack_.emplace_back());
@@ -566,10 +567,10 @@ const sequence_symbol* hlasm_context::get_opencode_sequence_symbol(id_index name
     return nullptr;
 }
 
-void hlasm_context::set_branch_counter(A_t value)
+size_t hlasm_context::set_branch_counter(A_t value)
 {
     curr_scope()->branch_counter = value;
-    ++curr_scope()->branch_counter_change;
+    return ++curr_scope()->branch_counter_change;
 }
 
 int hlasm_context::get_branch_counter() const { return curr_scope()->branch_counter; }

parser_library/src/context/hlasm_context.h

@@ -100,6 +100,8 @@ class hlasm_context
     std::unique_ptr<using_collection> m_usings;
     std::vector<index_t<using_collection>> m_active_usings;
 
+    long long m_statements_remaining;
+
 public:
     hlasm_context(std::string file_name = "",
         asm_option asm_opts = {},
@@ -180,7 +182,7 @@ class hlasm_context
     // returns nullptr if there is none
     const sequence_symbol* get_opencode_sequence_symbol(id_index name) const;
 
-    void set_branch_counter(A_t value);
+    size_t set_branch_counter(A_t value);
     A_t get_branch_counter() const;
     void decrement_branch_counter();
 
@@ -314,6 +316,8 @@ class hlasm_context
 
     using name_result = std::pair<bool, context::id_index>;
     name_result try_get_symbol_name(const std::string& symbol);
+
+    bool next_statement() { return --m_statements_remaining >= 0; }
 };
 
 bool test_symbol_for_read(

parser_library/src/diagnostic.cpp

@@ -1980,7 +1980,7 @@ diagnostic_op diagnostic_op::error_E055(const range& range)
 
 diagnostic_op diagnostic_op::error_E056(const range& range)
 {
-    return diagnostic_op(diagnostic_severity::error, "E055", "ACTR counter exceeded", range);
+    return diagnostic_op(diagnostic_severity::error, "E056", "ACTR counter exceeded", range);
 }
 
 diagnostic_op diagnostic_op::error_E057(const range& range)
@@ -2016,9 +2016,10 @@ diagnostic_op diagnostic_op::error_E062(const range& range)
     return diagnostic_op(diagnostic_severity::error, "E062", "Recursive COPY", range);
 }
 
-diagnostic_op diagnostic_op::error_E063(const range& range)
+diagnostic_op diagnostic_op::error_W063(const range& range)
 {
-    return diagnostic_op(diagnostic_severity::error, "E063", "Too many ACTR calls, exiting", range);
+    return diagnostic_op(
+        diagnostic_severity::warning, "W063", "Too many ACTR calls, review the use of the ACTR instruction", range);
 }
 
 diagnostic_op diagnostic_op::error_E064(const range& range)
@@ -2096,6 +2097,11 @@ diagnostic_op diagnostic_op::error_E076(const range& range)
         diagnostic_severity::error, "E076", "SYSLIST must be subscripted; using default subscript is 1", range);
 }
 
+diagnostic_op diagnostic_op::error_E077(const range& range)
+{
+    return diagnostic_op(diagnostic_severity::error, "E077", "Statement count limit reached", range);
+}
+
 diagnostic_op diagnostic_op::warning_W010(std::string_view message, const range& range)
 {
     return diagnostic_op(diagnostic_severity::warning, "W010", concat(message, " not expected"), range);

parser_library/src/diagnostic.h

@@ -571,7 +571,7 @@ struct diagnostic_op
 
     static diagnostic_op error_E062(const range& range);
 
-    static diagnostic_op error_E063(const range& range);
+    static diagnostic_op error_W063(const range& range);
 
     static diagnostic_op error_E064(const range& range);
 
@@ -599,6 +599,8 @@ struct diagnostic_op
 
     static diagnostic_op error_E076(const range& range);
 
+    static diagnostic_op error_E077(const range& range);
+
     static diagnostic_op warning_W010(std::string_view message, const range& range);
 
     static diagnostic_op warning_W011(const range& range);

parser_library/src/parsing/grammar/ca_expr_rules.g4

@@ -95,11 +95,11 @@ term returns [ca_expr_ptr ca_expr]
 		auto r = provider.get_range($self_def_term.ctx);
 		$ca_expr = std::make_unique<ca_constant>($self_def_term.value, r);
 	}
-	| num
+	| signed_num
 	{
-		collector.add_hl_symbol(token_info(provider.get_range( $num.ctx),hl_scopes::number));
-		auto r = provider.get_range($num.ctx);
-		$ca_expr = std::make_unique<ca_constant>($num.value, r);
+		collector.add_hl_symbol(token_info(provider.get_range( $signed_num.ctx),hl_scopes::number));
+		auto r = provider.get_range($signed_num.ctx);
+		$ca_expr = std::make_unique<ca_constant>($signed_num.value, r);
 	}
 	| ca_dupl_factor id_no_dot subscript_ne
 	{

parser_library/src/parsing/grammar/hlasmparser.g4

@@ -180,6 +180,12 @@ num_ch
 num returns [self_def_t value]
 	: num_ch									{$value = parse_self_def_term("D",$num_ch.ctx->getText(),provider.get_range($num_ch.ctx));};
 
+signed_num_ch
+	: MINUS? NUM+;
+
+signed_num returns [self_def_t value]
+	: signed_num_ch									{$value = parse_self_def_term("D",$signed_num_ch.ctx->getText(),provider.get_range($signed_num_ch.ctx));};
+
 self_def_term returns [self_def_t value]
 	: ORDSYMBOL string							
 	{

parser_library/src/processing/instruction_sets/ca_processor.cpp

@@ -263,7 +263,7 @@ bool ca_processor::prepare_ACTR(const semantics::complete_statement& stmt, conte
     const auto* ca_op = stmt.operands_ref().value[0]->access_ca();
     assert(ca_op);
 
-    if (ca_op->kind == semantics::ca_kind::EXPR || ca_op->kind == semantics::ca_kind::VAR)
+    if (ca_op->kind == semantics::ca_kind::EXPR)
     {
         ctr = ca_op->access_expr()->expression->evaluate<context::A_t>(eval_ctx);
         return true;
@@ -279,11 +279,14 @@ void ca_processor::process_ACTR(const semantics::complete_statement& stmt)
 {
     register_seq_sym(stmt);
 
-    context::A_t ctr;
-    bool ok = prepare_ACTR(stmt, ctr);
-
-    if (ok)
-        hlasm_ctx.set_branch_counter(ctr);
+    if (context::A_t ctr; prepare_ACTR(stmt, ctr))
+    {
+        static constexpr size_t ACTR_LIMIT = 1000;
+        if (hlasm_ctx.set_branch_counter(ctr) == ACTR_LIMIT)
+        {
+            add_diagnostic(diagnostic_op::error_W063(stmt.stmt_range_ref()));
+        }
+    }
 }
 
 bool ca_processor::prepare_AGO(const semantics::complete_statement& stmt,

parser_library/src/processing/statement_processors/ordinary_processor.cpp

@@ -273,6 +273,13 @@ void ordinary_processor::check_postponed_statements(
 
 bool ordinary_processor::check_fatals(range line_range)
 {
+    if (!hlasm_ctx.next_statement())
+    {
+        add_diagnostic(diagnostic_op::error_E077(line_range));
+        finished_flag_ = true;
+        return true;
+    }
+
     if (hlasm_ctx.scope_stack().size() > NEST_LIMIT)
     {
         while (hlasm_ctx.is_in_macro())
@@ -293,13 +300,6 @@ bool ordinary_processor::check_fatals(range line_range)
         return true;
     }
 
-    if (hlasm_ctx.scope_stack().back().branch_counter_change > ACTR_LIMIT)
-    {
-        add_diagnostic(diagnostic_op::error_E063(line_range));
-        finished_flag_ = true;
-        return true;
-    }
-
     return false;
 }
 

parser_library/src/processing/statement_processors/ordinary_processor.h

@@ -29,7 +29,6 @@ namespace hlasm_plugin::parser_library::processing {
 class ordinary_processor : public statement_processor
 {
     static constexpr size_t NEST_LIMIT = 100;
-    static constexpr size_t ACTR_LIMIT = 1000;
 
     expressions::evaluation_context eval_ctx;
 

parser_library/test/expressions/arithmetic_expression_test.cpp

@@ -288,3 +288,20 @@ TEST(arithmetic_expressions, dots)
         ASSERT_EQ(a.diags().empty(), ok);
     }
 }
+
+TEST(arithmetic_expressions, limits)
+{
+    std::string input =
+        R"(
+&A SETA 2147483647
+&B SETA -2147483648
+)";
+    analyzer a(input);
+    a.analyze();
+
+    a.collect_diags();
+    EXPECT_TRUE(a.diags().empty());
+
+    EXPECT_EQ(get_var_value<A_t>(a.hlasm_ctx(), "A"), SET_t(2147483647));
+    EXPECT_EQ(get_var_value<A_t>(a.hlasm_ctx(), "B"), SET_t(static_cast<A_t>(-2147483648)));
+}

parser_library/test/expressions/ca_function_test.cpp

@@ -298,15 +298,6 @@ TEST_P(ca_func, test)
 
     if (!GetParam().erroneous)
     {
-        ASSERT_EQ(result.type, GetParam().true_result.type);
-
-        if (result.type == context::SET_t_enum::A_TYPE)
-            EXPECT_EQ(result.access_a(), GetParam().true_result.access_a());
-        else if (result.type == context::SET_t_enum::B_TYPE)
-            EXPECT_EQ(result.access_b(), GetParam().true_result.access_b());
-        else if (result.type == context::SET_t_enum::C_TYPE)
-            EXPECT_EQ(result.access_c(), GetParam().true_result.access_c());
-        else
-            FAIL();
+        EXPECT_EQ(result, GetParam().true_result);
     }
 }

parser_library/test/expressions/logical_expression_test.cpp

@@ -33,15 +33,17 @@ TEST(logical_expressions, valid_assignment)
         R"(
 &A1 SETB 1
 &A2 SETB 0
+&A3 SETB (&A3)
 )";
     analyzer a(input);
     a.analyze();
 
     a.collect_diags();
-    ASSERT_EQ(a.diags().size(), (size_t)0);
+    EXPECT_TRUE(a.diags().empty());
 
     SETBEQ("A1", 1);
     SETBEQ("A2", 0);
+    SETBEQ("A3", 0);
 }
 
 TEST(logical_expressions, empty_string_conversion)
@@ -64,15 +66,14 @@ TEST(logical_expressions, invalid_assignment)
     std::string input =
         R"(
 &A1 SETB (C'D')
-&A2 SETB (&A2)
-&A3 SETB (10)
-&A4 SETB (-1)
+&A2 SETB (10)
+&A3 SETB (-1)
 )";
     analyzer a(input);
     a.analyze();
 
     a.collect_diags();
-    ASSERT_EQ(a.diags().size(), (size_t)4);
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "CE004", "CE004", "CE004" }));
 }
 
 TEST(logical_expressions, invalid_expression)

parser_library/test/processing/ca_instr_test.cpp

@@ -376,7 +376,7 @@ TEST(ACTR, exceeded)
 
     a.collect_diags();
 
-    ASSERT_EQ(a.diags().size(), (size_t)1);
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "E056" }));
 }
 
 TEST(ACTR, infinite_ACTR)
@@ -386,13 +386,31 @@ TEST(ACTR, infinite_ACTR)
  ACTR 1024
  LR 1,1
  AGO .A
+)");
+    analyzer a(input, analyzer_options(asm_option { .statement_count_limit = 10000 }));
+    a.analyze();
+
+    a.collect_diags();
+
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "W063", "E077" }));
+}
+
+TEST(ACTR, negative)
+{
+    std::string input(R"(
+&A SETA -2147483648
+   ACTR &A
+   AGO .A
+.A ANOP
+&B SETA 1
 )");
     analyzer a(input);
     a.analyze();
 
     a.collect_diags();
 
-    ASSERT_EQ(a.diags().size(), (size_t)1);
+    EXPECT_TRUE(matches_message_codes(a.diags(), { "E056" }));
+    EXPECT_EQ(get_var_value<A_t>(a.hlasm_ctx(), "B"), std::nullopt);
 }
 
 TEST(MHELP, SYSNDX_limit)