Merge pull request #4117 from iguessthislldo/igtd/dynamic-data-adapter

Fix Warnings and Typedef of Typedef Build Issues

Author: iguessthislldo

dds/DCPS/Definitions.h

@@ -83,6 +83,12 @@
 #  define OPENDDS_HAS_EXPLICIT_INTS 0
 #endif
 
+#if defined __GNUC__ && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || defined __clang__)
+#  define OPENDDS_GCC_HAS_DIAG_PUSHPOP 1
+#else
+#  define OPENDDS_GCC_HAS_DIAG_PUSHPOP 0
+#endif
+
 OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL
 
 namespace OpenDDS {

dds/DCPS/DirentWrapper.h

@@ -1,8 +1,9 @@
 #ifndef OPENDDS_DCPS_DIRENTWRAPPER_H
 #define OPENDDS_DCPS_DIRENTWRAPPER_H
 
-#if defined __GNUC__ && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 5) || defined __clang__)
-#  define OPENDDS_GCC_HAS_DIAG_PUSHPOP
+#include "Definitions.h"
+
+#if OPENDDS_GCC_HAS_DIAG_PUSHPOP
 #  pragma GCC diagnostic push
 #  if defined(__has_warning)
 #    if __has_warning("-Wdeprecated-declarations")
@@ -13,7 +14,7 @@
 #  endif
 #endif
 #include <ace/Dirent.h>
-#ifdef OPENDDS_GCC_HAS_DIAG_PUSHPOP
+#if OPENDDS_GCC_HAS_DIAG_PUSHPOP
 #  pragma GCC diagnostic pop
 #endif
 

dds/DCPS/XTypes/DynamicDataAdapter.h

@@ -527,6 +527,16 @@ class OpenDDS_Dcps_Export DynamicDataAdapter : public DynamicDataBase {
     return rc;
   }
 
+  /*
+   * It's possible to call this function with incorrectly matched types and
+   * GCC could warn about this when the source value from set_*_value is
+   * smaller than T in get_raw_value. This isn't actually a problem because
+   * check_member requires tk to match the member's type.
+   */
+#  if OPENDDS_GCC_HAS_DIAG_PUSHPOP
+#    pragma GCC diagnostic push
+#    pragma GCC diagnostic ignored "-Warray-bounds"
+#  endif
   template <typename T>
   DDS::ReturnCode_t set_simple_raw_value(
     const char* method, T& dest, DDS::MemberId id, const void* source, DDS::TypeKind tk)
@@ -537,6 +547,9 @@ class OpenDDS_Dcps_Export DynamicDataAdapter : public DynamicDataBase {
     }
     return rc;
   }
+#  if OPENDDS_GCC_HAS_DIAG_PUSHPOP
+#    pragma GCC diagnostic pop
+#  endif
 
   // A reference to a std::vector<bool> element isn't a bool&, so it's a special
   // case.

dds/idl/dds_generator.cpp

@@ -322,6 +322,7 @@ NestedForLoops::~NestedForLoops()
 string type_to_default_array(const std::string& indent, AST_Type* type, const string& name,
   bool is_anonymous, bool is_union, bool use_cxx11, Classification fld_cls)
 {
+  // TODO: Most of what's in here looks like it could be replaced with RefWrapper
   string val;
   string temp = name;
   if (temp.size() > 2 && temp.substr(temp.size() - 2, 2) == "()") {
@@ -332,7 +333,7 @@ string type_to_default_array(const std::string& indent, AST_Type* type, const st
   replace(temp.begin(), temp.end(), '[', '_');
   replace(temp.begin(), temp.end(), ']', '_');
   if (use_cxx11) {
-    string n = scoped(type->name());
+    string n = scoped(deepest_named_type(type)->name());
     if (is_anonymous) {
       n = n.substr(0, n.rfind("::") + 2) + "AnonymousType_" + type->local_name()->get_string();
       n = (fld_cls == AST_Decl::NT_sequence) ? (n + "_seq") : n;
@@ -423,14 +424,28 @@ string type_to_default(const std::string& indent, AST_Type* type, const string&
 
 std::string field_type_name(AST_Field* field, AST_Type* field_type)
 {
-  std::string name;
+  if (!field_type) {
+    field_type = field->field_type();
+  }
   const Classification cls = classify(field_type);
-  name = (cls & CL_STRING) ? string_type(cls) : scoped(field_type->name());
+  const std::string name = (cls & CL_STRING) ?
+    string_type(cls) : scoped(deepest_named_type(field_type)->name());
   if (field) {
     FieldInfo af(*field);
     if (af.as_base_ && af.type_->anonymous()) {
-      name = af.scoped_type_;
+      return af.scoped_type_;
     }
   }
   return name;
 }
+
+AST_Type* deepest_named_type(AST_Type* type)
+{
+  AST_Type* consider = type;
+  AST_Type* named_type = type;
+  while (consider->node_type() == AST_Decl::NT_typedef) {
+    named_type = consider;
+    consider = dynamic_cast<AST_Typedef*>(named_type)->base_type();
+  }
+  return named_type;
+}

dds/idl/dds_generator.h

@@ -786,7 +786,15 @@ struct Intro {
   }
 };
 
-std::string field_type_name(AST_Field* field, AST_Type* field_type);
+std::string field_type_name(AST_Field* field, AST_Type* field_type = 0);
+
+/**
+ * For the some situations, like a tag name, the type name we need is the
+ * deepest named type, not the actual type. This will be the name of the
+ * deepest typedef if it's an array or sequence, otherwise the name of the
+ * type.
+ */
+AST_Type* deepest_named_type(AST_Type* type);
 
 typedef std::string (*CommonFn)(
   const std::string& indent, AST_Decl* node,
@@ -834,7 +842,7 @@ void generateCaseBody(
       rhs = use_cxx11 ? "tmp" : "tmp.out()";
     } else if (use_cxx11 && (br_cls & (CL_ARRAY | CL_SEQUENCE))) {  //array or seq C++11
       rhs = "IDL::DistinctType<" + brType + ", "
-        + dds_generator::get_tag_name(dds_generator::scoped_helper(branch->field_type()->name(), "::"))
+        + dds_generator::get_tag_name(brType)
         + ">(tmp)";
     } else if (br_cls & CL_ARRAY) { //array classic
       forany = "    " + brType + "_forany fa = tmp;\n";

dds/idl/dynamic_data_adapter_generator.cpp

@@ -249,16 +249,7 @@ namespace {
         "    }\n";
     } else if (seq_node || array_node) {
       AST_Type* const base_type = seq_node ? seq_node->base_type() : array_node->base_type();
-
-      // For the type name we need the deepest named type, not the actual type.
-      // This will be the name of the deepest typedef if it's an array or
-      // sequence, otherwise the name of the type.
-      AST_Type* consider = base_type;
-      AST_Type* named_type = base_type;
-      while (consider->node_type() == AST_Decl::NT_typedef) {
-        named_type = consider;
-        consider = dynamic_cast<AST_Typedef*>(named_type)->base_type();
-      }
+      AST_Type* const named_type = deepest_named_type(base_type);
 
       std::string op_type;
       std::string extra_access;

dds/idl/marshal_generator.cpp

@@ -453,7 +453,7 @@ namespace {
       }
     } else {
       Intro intro;
-      RefWrapper wrapper(seq->base_type(), scoped(seq->base_type()->name()),
+      RefWrapper wrapper(seq->base_type(), scoped(deepest_named_type(seq->base_type())->name()),
         classic_array_copy ? tempvar : stream_to, false);
       wrapper.classic_array_copy_ = classic_array_copy;
       wrapper.done(&intro);
@@ -523,7 +523,7 @@ namespace {
     }
 
     const std::string cxx_elem =
-      anonymous ? anonymous->scoped_elem_ : scoped(seq->base_type()->name());
+      anonymous ? anonymous->scoped_elem_ : scoped(deepest_named_type(seq->base_type())->name());
     const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11;
 
     RefWrapper(base_wrapper).done().generate_tag();
@@ -804,7 +804,7 @@ namespace {
         std::string classic_array_copy;
         if (!use_cxx11 && (elem_cls & CL_ARRAY)) {
           RefWrapper classic_array_wrapper(
-            seq->base_type(), scoped(seq->base_type()->name()), elem_access);
+            seq->base_type(), scoped(deepest_named_type(seq->base_type())->name()), elem_access);
           classic_array_wrapper.classic_array_copy_ = true;
           classic_array_wrapper.done(&intro);
           classic_array_copy = classic_array_wrapper.classic_array_copy();
@@ -932,7 +932,7 @@ namespace {
       be_global->add_referenced(elem->file_name().c_str());
     }
     const std::string cxx_elem =
-      anonymous ? anonymous->scoped_elem_ : scoped(arr->base_type()->name());
+      anonymous ? anonymous->scoped_elem_ : scoped(deepest_named_type(arr->base_type())->name());
     const ACE_CDR::ULong n_elems = array_element_count(arr);
 
     RefWrapper(base_wrapper).done().generate_tag();
@@ -1077,7 +1077,8 @@ namespace {
           std::string classic_array_copy;
           if (!use_cxx11 && (elem_cls & CL_ARRAY)) {
             RefWrapper classic_array_wrapper(
-              arr->base_type(), scoped(arr->base_type()->name()), wrapper.value_access() + nfl.index_);
+              arr->base_type(), scoped(deepest_named_type(arr->base_type())->name()),
+              wrapper.value_access() + nfl.index_);
             classic_array_wrapper.classic_array_copy_ = true;
             classic_array_wrapper.done(&intro);
             classic_array_copy = classic_array_wrapper.classic_array_copy();
@@ -3130,7 +3131,6 @@ marshal_generator::gen_field_getValueFromSerialized(AST_Structure* node, const s
   for (Fields::Iterator i = fields.begin(); i != fields_end; ++i) {
     AST_Field* const field = *i;
     size_t size = 0;
-    const std::string field_name = field->local_name()->get_string();
     const std::string idl_name = canonical_name(field);
     AST_Type* const field_type = resolveActualType(field->field_type());
     Classification fld_cls = classify(field_type);
@@ -3183,7 +3183,7 @@ marshal_generator::gen_field_getValueFromSerialized(AST_Structure* node, const s
           "      return getMetaStruct<" + scoped(field_type->name()) + ">().getValue(strm, subfield.c_str());\n"
           "    } else {\n"
           "      if (!gen_skip_over(strm, static_cast<" + scoped(field_type->name()) + "*>(0))) {\n"
-          "        throw std::runtime_error(\"Field '" + field_name + "' could not be skipped\");\n"
+          "        throw std::runtime_error(\"Field '" + idl_name + "' could not be skipped\");\n"
           "      }\n"
           "    }\n";
     } else { // array, sequence, union:
@@ -3192,10 +3192,10 @@ marshal_generator::gen_field_getValueFromSerialized(AST_Structure* node, const s
         post = "_forany";
       } else if (use_cxx11 && (fld_cls & (CL_ARRAY | CL_SEQUENCE))) {
         pre = "IDL::DistinctType<";
-        post = ", " + dds_generator::get_tag_name(scoped(field->field_type()->name())) + ">";
+        post = ", " + dds_generator::get_tag_name(scoped(deepest_named_type(field->field_type())->name())) + ">";
       }
       const std::string ptr = field->field_type()->anonymous() ?
-        FieldInfo(*field).ptr_ : (pre + scoped(field->field_type()->name()) + post + '*');
+        FieldInfo(*field).ptr_ : (pre + field_type_name(field) + post + '*');
       expr +=
         "    if (!gen_skip_over(strm, static_cast<" + ptr + ">(0))) {\n"
         "      throw std::runtime_error(\"Field \" + OPENDDS_STRING(field) + \" could not be skipped\");\n"
@@ -3733,7 +3733,6 @@ bool marshal_generator::gen_union(AST_Union* node, UTL_ScopedName* name,
       be_global->impl_ <<
         "  " << scoped(discriminator->name()) << " disc;\n" <<
         streamAndCheck(">> " + getWrapper("disc", discriminator, WD_INPUT));
-      be_global->impl_ << "/* HERE */\n";
       if (generateSwitchForUnion(node, "disc", streamCommon, branches,
           discriminator, "", ">> ", cxx.c_str())) {
         be_global->impl_ <<

dds/idl/metaclass_generator.cpp

@@ -475,7 +475,7 @@ void generate_anon_fields(AST_Structure* node)
               post = "_forany";
             } else if (use_cxx11 && (elem_cls & (CL_ARRAY | CL_SEQUENCE))) {
               pre = "IDL::DistinctType<";
-              post = ", " + dds_generator::get_tag_name(dds_generator::scoped_helper(elem_orig->name(), "_")) + ">";
+              post = ", " + dds_generator::get_tag_name(dds_generator::scoped_helper(deepest_named_type(elem_orig)->name(), "_")) + ">";
             }
             be_global->impl_ <<
               "    if (!gen_skip_over(ser, static_cast<" << pre << cxx_elem << post
@@ -597,7 +597,7 @@ metaclass_generator::gen_typedef(AST_Typedef*, UTL_ScopedName* name,
         post = "_forany";
       } else if (use_cxx11 && (elem_cls & (CL_ARRAY | CL_SEQUENCE))) {
         pre = "IDL::DistinctType<";
-        post = ", " + dds_generator::get_tag_name(scoped_helper(elem_orig->name(), "::")) + ">";
+        post = ", " + dds_generator::get_tag_name(scoped_helper(deepest_named_type(elem_orig)->name(), "::")) + ">";
       }
       be_global->impl_ <<
         "    if (!gen_skip_over(ser, static_cast<" << pre << cxx_elem << post
@@ -637,7 +637,7 @@ std::string gen_union_branch(const std::string&, AST_Decl* branch, const std::st
       post = "_forany";
     } else if (use_cxx11 && (br_cls & (CL_ARRAY | CL_SEQUENCE))) {
       pre = "IDL::DistinctType<";
-      post = ", " + dds_generator::get_tag_name(dds_generator::scoped_helper(br_type->name(), "::")) + ">";
+      post = ", " + dds_generator::get_tag_name(dds_generator::scoped_helper(deepest_named_type(br_type)->name(), "::")) + ">";
     }
     ss <<
       "    if (!gen_skip_over(ser, static_cast<" << pre

tests/DCPS/Compiler/idl_test3_lib/FooDef.idl

@@ -58,6 +58,31 @@ module Xyz {
   typedef sequence<SeqOfLong,4>    SeqOfSeqOfLong;
   typedef sequence<SeqOfAnEnum,4>  SeqOfSeqOfAnEnum;
 
+  // typedefs of typedefs
+  typedef ArrayOfLong TypedefOfArrayOfLong;
+  typedef SeqOfArrayOfLong TypedefOfSeqOfArrayOfLong;
+  typedef sequence<TypedefOfArrayOfLong, 3> SeqOfTypedefOfArrayOfLong;
+  typedef TypedefOfSeqOfArrayOfLong ArrayOfTypedefOfSeqOfArrayOfLong[2];
+
+  @topic
+  struct StructOfTypedefsOfTypedefs {
+    TypedefOfArrayOfLong typedef_of_array_of_long;
+    TypedefOfSeqOfArrayOfLong typedef_of_seq_of_array_of_long;
+    SeqOfTypedefOfArrayOfLong seq_of_typedef_of_array_of_long;
+    ArrayOfTypedefOfSeqOfArrayOfLong array_of_typedef_of_seq_of_array_of_long;
+  };
+
+  union UnionOfTypedefsOfTypedefs switch (int32) {
+  case 0:
+    TypedefOfArrayOfLong typedef_of_array_of_long;
+  case 1:
+    TypedefOfSeqOfArrayOfLong typedef_of_seq_of_array_of_long;
+  case 2:
+    SeqOfTypedefOfArrayOfLong seq_of_typedef_of_array_of_long;
+  default:
+    ArrayOfTypedefOfSeqOfArrayOfLong array_of_typedef_of_seq_of_array_of_long;
+  };
+
   // typedef of a "special" scalar type
   typedef octet OctetTypedef;
 

tests/unit-tests/dds/DCPS/XTypes/DynamicDataAdapter.cpp

@@ -188,8 +188,8 @@ void test_collection_struct(T& col_struct)
   EXPECT_EQ(col_struct.f32[1], 0.11f);
   EXPECT_EQ(col_struct.f64[0], 0.20);
   EXPECT_EQ(col_struct.f64[1], 0.22);
-  EXPECT_EQ(col_struct.f128[0], 0.30);
-  EXPECT_EQ(col_struct.f128[1], 0.33);
+  EXPECT_EQ(static_cast<double>(col_struct.f128[0]), 0.30);
+  EXPECT_EQ(static_cast<double>(col_struct.f128[1]), 0.33);
   EXPECT_EQ(col_struct.c8[0], '1');
   EXPECT_EQ(col_struct.c8[1], '2');
   EXPECT_EQ(col_struct.c16[0], L'1');
@@ -240,6 +240,11 @@ TEST_F(dds_DCPS_XTypes_DynamicDataAdapter, test_struct)
     set_collection_struct(x.anon_seqs);
     set_collection_struct(x.arrays);
     set_collection_struct(x.anon_arrays);
+    x.typedef_of_seq_typedef.length(2);
+    x.typedef_of_seq_typedef[0] = 1;
+    x.typedef_of_seq_typedef[1] = 2;
+    x.typedef_of_array_typedef[0] = 1;
+    x.typedef_of_array_typedef[1] = 2;
 
     DDS::DynamicData_var dda = get_dynamic_data_adapter<TestStruct>(dt, x);
     ASSERT_RC_OK(copy(ddi, dda));
@@ -261,7 +266,7 @@ TEST_F(dds_DCPS_XTypes_DynamicDataAdapter, test_struct)
     EXPECT_EQ(y.u64, 8u);
     EXPECT_EQ(y.f32, 0.1f);
     EXPECT_EQ(y.f64, 0.2);
-    EXPECT_EQ(y.f128, 0.3);
+    EXPECT_EQ(static_cast<double>(y.f128), 0.3);
     EXPECT_EQ(y.c8, '!');
     EXPECT_EQ(y.c16, L'@');
     EXPECT_STREQ(y.s8, "Hello");
@@ -275,6 +280,11 @@ TEST_F(dds_DCPS_XTypes_DynamicDataAdapter, test_struct)
     test_collection_struct(y.anon_seqs);
     test_collection_struct(y.arrays);
     test_collection_struct(y.anon_arrays);
+    ASSERT_EQ(y.typedef_of_seq_typedef.length(), 2u);
+    EXPECT_EQ(y.typedef_of_seq_typedef[0], 1);
+    EXPECT_EQ(y.typedef_of_seq_typedef[1], 2);
+    EXPECT_EQ(y.typedef_of_array_typedef[0], 1);
+    EXPECT_EQ(y.typedef_of_array_typedef[1], 2);
   }
 }