fix: N' attribute and subscript array contents (#249)

(closes #231)

clients/vscode-hlasmplugin/CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## ****Unreleased****
 
+#### Added
+
+#### Fixed
+- 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
+
 ## [1.1.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.0.0...1.1.0) (2022-03-29)
 
 #### Added

parser_library/src/context/macro_param_data.cpp

@@ -68,8 +68,12 @@ size_t macro_param_data_composite::size() const { return data_.size(); }
 
 
 macro_param_data_composite::macro_param_data_composite(std::vector<macro_data_ptr> value)
-    : macro_param_data_component(value.size())
-    , data_(move(value))
+    : macro_param_data_component(!value.empty() ? value.size() : 1)
+    , data_(!value.empty() ? std::move(value) : [] {
+        std::vector<context::macro_data_ptr> vec;
+        vec.push_back(std::make_unique<context::macro_param_data_dummy>());
+        return vec;
+    }())
 {
     value_.append("(");
     for (size_t i = 0; i < data_.size(); ++i)

parser_library/src/debugging/macro_param_variable.cpp

@@ -25,11 +25,15 @@ macro_param_variable::macro_param_variable(const context::macro_param_base& para
     , index_(std::move(index))
 {
     if (!index_.empty())
+    {
         name_ = std::to_string(index_.back());
+        value_ = macro_param_.get_value(index_);
+    }
     else
+    {
         name_ = "&" + *macro_param_.id;
-
-    value_ = macro_param_.get_value(index_);
+        value_ = macro_param_.macro_param_base::get_value();
+    }
 }
 
 const std::string& macro_param_variable::get_name() const { return name_; }

parser_library/src/debugging/set_symbol_variable.cpp

@@ -20,32 +20,20 @@
 using namespace hlasm_plugin::parser_library;
 using namespace hlasm_plugin::parser_library::debugging;
 
-void set_symbol_variable::fill_string_value()
-{
-    if (!is_scalar())
-        value_ = "";
-    else if (type() == set_type::A_TYPE)
-        value_ = std::to_string(get_value<context::A_t>());
-    else if (type() == set_type::B_TYPE)
-        value_ = get_value<context::B_t>() ? "TRUE" : "FALSE";
-    else
-        value_ = get_value<std::string>();
-}
-
 set_symbol_variable::set_symbol_variable(const context::set_symbol_base& set_sym, int index)
     : set_symbol_(set_sym)
     , index_(index)
+    , name_(std::to_string(index + 1))
 {
-    name_ = std::to_string(index + 1);
-    fill_string_value();
+    value_ = get_string_value(index_);
 }
 
 set_symbol_variable::set_symbol_variable(const context::set_symbol_base& set_sym)
     : set_symbol_(set_sym)
     , index_()
+    , name_("&" + *set_symbol_.id)
 {
-    name_ = "&" + *set_symbol_.id;
-    fill_string_value();
+    value_ = get_string_value(index_);
 }
 
 const std::string& set_symbol_variable::get_name() const { return name_; }
@@ -67,22 +55,50 @@ std::vector<variable_ptr> set_symbol_variable::values() const
 {
     std::vector<std::unique_ptr<debugging::variable>> vals;
 
-    auto keys = set_symbol_.keys();
-    for (size_t i = 0; i < keys.size(); ++i)
-        vals.push_back(std::make_unique<set_symbol_variable>(set_symbol_, (int)keys[i]));
+    for (auto keys = set_symbol_.keys(); const auto& key : keys)
+        vals.push_back(std::make_unique<set_symbol_variable>(set_symbol_, static_cast<int>(key)));
 
     return vals;
 }
 
 size_t set_symbol_variable::size() const { return set_symbol_.size(); }
 
+std::string set_symbol_variable::get_string_value(const std::optional<int>& index) const
+{
+    if (!index && !is_scalar())
+        return get_string_array_value();
+    else if (type() == set_type::A_TYPE)
+        return std::to_string(get_value<context::A_t>(index));
+    else if (type() == set_type::B_TYPE)
+        return get_value<context::B_t>(index) ? "TRUE" : "FALSE";
+    else
+        return get_value<context::C_t>(index);
+}
+
+std::string set_symbol_variable::get_string_array_value() const
+{
+    std::string array_value;
+    array_value.append("(");
+
+    auto keys = set_symbol_.keys();
+    assert(!keys.empty());
+
+    for (const auto& key : keys)
+    {
+        array_value.append(get_string_value(static_cast<int>(key)));
+        array_value.append(",");
+    }
+
+    array_value.back() = ')';
+
+    return array_value;
+}
+
 template<typename T>
-inline T set_symbol_variable::get_value() const
+inline T set_symbol_variable::get_value(const std::optional<int> index) const
 {
-    if (set_symbol_.is_scalar)
-        return set_symbol_.access_set_symbol<T>()->get_value();
-    else if (index_)
-        return set_symbol_.access_set_symbol<T>()->get_value(*index_);
+    if (!set_symbol_.is_scalar && index)
+        return set_symbol_.access_set_symbol<T>()->get_value(*index);
     else
         return set_symbol_.access_set_symbol<T>()->get_value();
-}
+}

parser_library/src/debugging/set_symbol_variable.h

@@ -42,14 +42,15 @@ class set_symbol_variable : public variable
 
 private:
     template<typename T>
-    T get_value() const;
+    T get_value(const std::optional<int> index) const;
 
-    void fill_string_value();
+    std::string get_string_value(const std::optional<int>& index) const;
+    std::string get_string_array_value() const;
 
     const context::set_symbol_base& set_symbol_;
     const std::optional<int> index_;
 
-    std::string name_;
+    const std::string name_;
     std::string value_;
 };
 

parser_library/test/context/CMakeLists.txt

@@ -18,6 +18,7 @@ target_sources(library_test PRIVATE
 	literals_test.cpp
 	macro_test.cpp
 	ord_sym_test.cpp
+	system_variable_subscripts_test.cpp
 	system_variable_test.cpp
 	using_test.cpp
 )

parser_library/test/context/context_test.cpp

@@ -218,6 +218,14 @@ TEST(context_macro_param, param_data)
     EXPECT_EQ(ptr->get_value(), "(first,second,third)");
 }
 
+TEST(context_macro_param, param_data_composite_empty)
+{
+    std::vector<macro_data_ptr> v;
+    macro_param_data_composite c(std::move(v));
+
+    EXPECT_EQ(c.get_value(), "()");
+}
+
 // testing more complex composite data
 TEST(context_macro_param, param_data_composite)
 {

parser_library/test/context/data_attribute_test.cpp

@@ -158,6 +158,42 @@ TEST(data_attributes, N_var_syms)
     ASSERT_EQ(a.diags().size(), (size_t)0);
 }
 
+TEST(data_attributes, N_var_syms_2)
+{
+    std::string input = R"(
+      MACRO
+      MAC_KEY_PAR &A=()
+      GBLA &VAR1
+&VAR1 SETA N'&A
+      MEND
+
+      MACRO
+      MAC_POS_PAR &A
+      GBLA &VAR2
+&VAR2 SETA N'&A
+      MEND
+
+      MACRO
+      MAC2 &A=()
+      GBLA &VAR1,&VAR2
+      MAC_KEY_PAR A=&A
+      MAC_POS_PAR &A
+      MEND
+
+      GBLA &VAR1,&VAR2
+      MAC2
+)";
+
+    analyzer a(input);
+    a.analyze();
+
+    EXPECT_EQ(get_var_value<A_t>(a.hlasm_ctx(), "VAR1"), 1);
+    EXPECT_EQ(get_var_value<A_t>(a.hlasm_ctx(), "VAR2"), 1);
+
+    a.collect_diags();
+    ASSERT_EQ(a.diags().size(), (size_t)0);
+}
+
 TEST(data_attributes, K_var_syms_good)
 {
     std::string input = R"(