uhd: Make make_args_t fully opaque

Before this change, UHD declared make_args_t publicly so that its
associated unique_ptr would not need a custom deleter. However, this
approach required defining a destructor for make_args_t all over the
place, and still caused linker issues for OOTs on Windows.

This change does what probably should have been done in the first place:
Make make_args_t fully opaque, but provide a custom deleter so that
there is no problems with unique_ptr objects.

In order to not break existing out-of-tree modules, the new, opaque type
is renamed to make_args_int_t (for 'internal'). The existing make_args_t
is declared deprecated and does nothing.

However, all the tests which declared the ~make_args_t() destructor to
avoid linker errors are updated, and no longer implement their own
destructors, as this is no longer necessary.

host/include/uhd/rfnoc/noc_block_base.hpp

@@ -28,8 +28,7 @@ namespace uhd { namespace rfnoc {
 class clock_iface;
 class mb_controller;
 
-/*!
- * The primary interface to a NoC block in the FPGA
+/*! The primary interface to a NoC block in the FPGA
  *
  * The block supports three types of data access:
  * - Low-level register access
@@ -37,21 +36,40 @@ class mb_controller;
  * - Action execution
  *
  * The main difference between this class and its parent is the direct access to
- * registers, and the NoC&block IDs.
+ * registers, and the NoC-block IDs.
  */
 class UHD_API noc_block_base : public node_t, public register_iface_holder
 {
 public:
-    /*! A shared pointer to allow easy access to this class and for
-     *  automatic memory management.
-     */
     using sptr = std::shared_ptr<noc_block_base>;
 
-    //! Forward declaration for the constructor arguments
-    struct make_args_t;
+    //! Dummy make_args_t class
+    //
+    // This allows OOTs to compile even if they were created prior to making
+    // make_args_t fully opaque.
+    //
+    // This is deprecated and will be removed in future UHD versions.
+    struct UHD_API make_args_t
+    {
+        ~make_args_t();
+    };
 
-    //! Opaque pointer to the constructor arguments
-    using make_args_ptr = std::unique_ptr<make_args_t>;
+    /*! Data structure to hold the arguments passed into the noc_block_base ctor
+     *
+     * This is a fully opaque data type - the struct definition is hidden in the
+     * implementation and not exposed in the public API. This allows the contents
+     * to be modified without breaking ABI compatibility.
+     */
+    struct make_args_int_t;
+
+    //! Custom deleter for make_args_int_t that doesn't require complete type
+    struct UHD_API make_args_deleter
+    {
+        void operator()(make_args_int_t* ptr);
+    };
+
+    //! Opaque pointer to the constructor arguments with custom deleter
+    using make_args_ptr = std::unique_ptr<make_args_int_t, make_args_deleter>;
 
     ~noc_block_base() override;
 
@@ -399,5 +417,3 @@ private:
 }; // class noc_block_base
 
 }} /* namespace uhd::rfnoc */
-
-#include <uhd/rfnoc/noc_block_make_args.hpp>

host/include/uhd/rfnoc/noc_block_make_args.hpp

@@ -5,64 +5,4 @@
 //
 
 #pragma once
-
-#include <uhd/property_tree.hpp>
-#include <uhd/rfnoc/noc_block_base.hpp>
-#include <uhd/rfnoc/register_iface.hpp>
-#include <uhd/rfnoc/rfnoc_types.hpp>
-
-namespace uhd { namespace rfnoc {
-
-class clock_iface;
-class mb_controller;
-
-/*! Data structure to hold the arguments passed into the noc_block_base ctor
- *
- * We want to hide these from the user, so she can't futz around with them.
- * Hence the opaque pointer, and non-UHD_API implementation.
- */
-struct noc_block_base::make_args_t
-{
-    ~make_args_t();
-
-    //! Noc-ID
-    noc_id_t noc_id;
-
-    //! Block ID (e.g. 0/Radio#0)
-    block_id_t block_id;
-
-    //! Number of input ports (gets reported from the FPGA)
-    size_t num_input_ports;
-
-    //! Number of output ports (gets reported from the FPGA)
-    size_t num_output_ports;
-
-    //! Value of the MTU register, converted to bytes
-    size_t mtu;
-
-    //! CHDR width of this block
-    chdr_w_t chdr_w;
-
-    //! Register interface to this block's register space
-    register_iface::sptr reg_iface;
-
-    //! Timebase clock interface object that is shared with the reg_iface
-    std::shared_ptr<clock_iface> tb_clk_iface;
-
-    //! Controlport clock interface object that is shared with the reg_iface
-    std::shared_ptr<clock_iface> ctrlport_clk_iface;
-
-    //! Reference to the motherboard controller associated with this block.
-    //
-    // Note that this may not be populated -- most blocks do not gain access to
-    // the motherboard controller.
-    std::shared_ptr<mb_controller> mb_control;
-
-    //! The subtree for this block
-    uhd::property_tree::sptr tree;
-
-    //! Additional args that can be parsed and used by this block
-    uhd::device_addr_t args;
-};
-
-}} /* namespace uhd::rfnoc */
+#pragma message "This header is deprecated and will be removed in future versions of UHD."

host/lib/include/uhdlib/rfnoc/noc_block_make_args.hpp

@@ -0,0 +1,69 @@
+//
+// Copyright 2019 Ettus Research, a National Instruments Brand
+//
+// SPDX-License-Identifier: GPL-3.0-or-later
+//
+
+#pragma once
+
+#include <uhd/property_tree.hpp>
+#include <uhd/rfnoc/noc_block_base.hpp>
+#include <uhd/rfnoc/register_iface.hpp>
+#include <uhd/rfnoc/rfnoc_types.hpp>
+
+namespace uhd { namespace rfnoc {
+
+class clock_iface;
+class mb_controller;
+
+/*! Data structure to hold the arguments passed into the noc_block_base ctor
+ *
+ * INTERNAL USE ONLY - This struct definition is available only to internal
+ * UHD library implementation files. External users only see the opaque pointer
+ * type defined in noc_block_base.hpp.
+ */
+struct noc_block_base::make_args_int_t
+{
+    ~make_args_int_t() = default;
+
+    //! Noc-ID
+    noc_id_t noc_id;
+
+    //! Block ID (e.g. 0/Radio#0)
+    block_id_t block_id;
+
+    //! Number of input ports (gets reported from the FPGA)
+    size_t num_input_ports;
+
+    //! Number of output ports (gets reported from the FPGA)
+    size_t num_output_ports;
+
+    //! Value of the MTU register, converted to bytes
+    size_t mtu;
+
+    //! CHDR width of this block
+    chdr_w_t chdr_w;
+
+    //! Register interface to this block's register space
+    register_iface::sptr reg_iface;
+
+    //! Timebase clock interface object that is shared with the reg_iface
+    std::shared_ptr<clock_iface> tb_clk_iface;
+
+    //! Controlport clock interface object that is shared with the reg_iface
+    std::shared_ptr<clock_iface> ctrlport_clk_iface;
+
+    //! Reference to the motherboard controller associated with this block.
+    //
+    // Note that this may not be populated -- most blocks do not gain access to
+    // the motherboard controller.
+    std::shared_ptr<mb_controller> mb_control;
+
+    //! The subtree for this block
+    uhd::property_tree::sptr tree;
+
+    //! Additional args that can be parsed and used by this block
+    uhd::device_addr_t args;
+};
+
+}} /* namespace uhd::rfnoc */

host/lib/rfnoc/mock_block.cpp

@@ -11,6 +11,7 @@
 #include <uhd/types/device_addr.hpp>
 #include <uhdlib/rfnoc/clock_iface.hpp>
 #include <uhdlib/rfnoc/factory.hpp>
+#include <uhdlib/rfnoc/noc_block_make_args.hpp>
 
 using namespace uhd::rfnoc;
 
@@ -31,7 +32,8 @@ uhd::rfnoc::mock_block_container uhd::rfnoc::get_mock_block(const noc_id_t noc_i
                                            : std::make_shared<mock_reg_iface_t>();
     ret_val.tree      = uhd::property_tree::make();
     // Create make args
-    ret_val.make_args                   = std::make_unique<noc_block_base::make_args_t>();
+    ret_val.make_args =
+        noc_block_base::make_args_ptr(new noc_block_base::make_args_int_t());
     ret_val.make_args->noc_id           = noc_id;
     ret_val.make_args->block_id         = block_id_t(fac_info.block_name);
     ret_val.make_args->num_input_ports  = num_inputs;

host/lib/rfnoc/noc_block_base.cpp

@@ -9,10 +9,15 @@
 #include <uhd/rfnoc/noc_block_base.hpp>
 #include <uhd/rfnoc/register_iface.hpp>
 #include <uhdlib/rfnoc/clock_iface.hpp>
+#include <uhdlib/rfnoc/noc_block_make_args.hpp>
 
 using namespace uhd::rfnoc;
 
-noc_block_base::make_args_t::~make_args_t() = default;
+// Custom deleter implementation for make_args_deleter
+void uhd::rfnoc::noc_block_base::make_args_deleter::operator()(make_args_int_t* ptr)
+{
+    delete ptr;
+}
 
 /******************************************************************************
  * Structors

host/lib/rfnoc/rfnoc_graph.cpp

@@ -9,12 +9,12 @@
 #include <uhd/rfnoc/defaults.hpp>
 #include <uhd/rfnoc/detail/graph.hpp>
 #include <uhd/rfnoc/mb_controller.hpp>
-#include <uhd/rfnoc/noc_block_make_args.hpp>
 #include <uhd/rfnoc/node.hpp>
 #include <uhd/rfnoc_graph.hpp>
 #include <uhdlib/rfnoc/block_container.hpp>
 #include <uhdlib/rfnoc/factory.hpp>
 #include <uhdlib/rfnoc/graph_stream_manager.hpp>
+#include <uhdlib/rfnoc/noc_block_make_args.hpp>
 #include <uhdlib/rfnoc/rfnoc_device.hpp>
 #include <uhdlib/rfnoc/rfnoc_rx_streamer.hpp>
 #include <uhdlib/rfnoc/rfnoc_tx_streamer.hpp>
@@ -756,9 +756,10 @@ private:
             auto block_reg_iface = _gsm->get_block_register_iface(
                 ctrl_sep_addr, portno, *ctrlport_clk_iface.get(), *tb_clk_iface.get());
             block_reg_iface->set_log_id(block_id.to_string());
-            auto make_args_uptr      = std::make_unique<noc_block_base::make_args_t>();
-            make_args_uptr->noc_id   = noc_id;
-            make_args_uptr->block_id = block_id;
+            auto make_args_uptr =
+                noc_block_base::make_args_ptr(new noc_block_base::make_args_int_t());
+            make_args_uptr->noc_id           = noc_id;
+            make_args_uptr->block_id         = block_id;
             make_args_uptr->num_input_ports  = block_info.num_inputs;
             make_args_uptr->num_output_ports = block_info.num_outputs;
             make_args_uptr->mtu =

host/tests/complex_gain_3000_test.cpp

@@ -16,9 +16,6 @@ using namespace uhd;
 using namespace uhd::cores;
 using namespace uhd::rfnoc;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 constexpr size_t NUM_CHANS    = 2;
 constexpr size_t NIPC         = 1;
 constexpr double TICK_RATE    = 125e6;

host/tests/rfnoc_block_tests/addsub_block_test.cpp

@@ -18,9 +18,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /* addsub_block_fixture is a class which is instantiated before each test
  * case is run. It sets up the block container and addsub_block_control
  * object, all of which are accessible to the test case. The instance of the

host/tests/rfnoc_block_tests/ddc_block_test.cpp

@@ -20,9 +20,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 namespace {
 
 constexpr size_t DEFAULT_MTU = 8000;

host/tests/rfnoc_block_tests/duc_block_test.cpp

@@ -19,9 +19,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 namespace {
 
 constexpr size_t DEFAULT_MTU = 8000;

host/tests/rfnoc_block_tests/ferrum_radio_mock.hpp

@@ -20,9 +20,6 @@ using namespace std::chrono_literals;
 using namespace uhd::usrp::fbx;
 using namespace uhd::experts;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 namespace {
 /* This class extends mock_reg_iface_t by adding a constructor that initializes
  * some of the read memory to contain the memory size for the radio block.

host/tests/rfnoc_block_tests/fft_block_test.cpp

@@ -16,9 +16,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t, adding a register poke override
  * that monitors the reset strobe address and sets a flag when written.

host/tests/rfnoc_block_tests/fft_block_test_v2.cpp

@@ -16,9 +16,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t, adding a register poke override
  * that monitors the reset strobe address and sets a flag when written.

host/tests/rfnoc_block_tests/fir_filter_block_test.cpp

@@ -18,9 +18,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t by adding poke and peek hooks that
  * monitor writes and reads to the registers implemented within the FIR

host/tests/rfnoc_block_tests/fosphor_block_test.cpp

@@ -17,9 +17,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t, handling three particular registers
  * (REG_ENABLE, REG_RANDOM, and REG_WF_CTRL) specially by allowing values

host/tests/rfnoc_block_tests/keep_one_in_n_test.cpp

@@ -16,9 +16,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t, adding a register peek override
  * that monitors when the WIDTH_N register is peek'd

host/tests/rfnoc_block_tests/lc_block_test.cpp

@@ -17,9 +17,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t by adding a constructor that initializes some of
  * the read memory to carry a valid compat number.

host/tests/rfnoc_block_tests/logpwr_block_test.cpp

@@ -19,9 +19,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 constexpr size_t NUM_CHANS = 4;
 
 /*

host/tests/rfnoc_block_tests/moving_average_block_test.cpp

@@ -19,9 +19,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 BOOST_AUTO_TEST_CASE(test_moving_average_block)
 {
     node_accessor_t node_accessor{};

host/tests/rfnoc_block_tests/null_block_test.cpp

@@ -18,9 +18,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 BOOST_AUTO_TEST_CASE(test_null_block)
 {
     node_accessor_t node_accessor{};

host/tests/rfnoc_block_tests/replay_block_test.cpp

@@ -20,9 +20,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 static const size_t CMD_Q_MAX = 32;
 
 /*

host/tests/rfnoc_block_tests/siggen_block_test.cpp

@@ -21,9 +21,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 constexpr size_t NUM_PORTS   = 4;
 constexpr size_t DEFAULT_MTU = 8000;
 

host/tests/rfnoc_block_tests/split_stream_block_test.cpp

@@ -16,9 +16,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 struct split_stream_block_fixture
 {
     //! Create a split stream block having a particular number of input and

host/tests/rfnoc_block_tests/switchboard_block_test.cpp

@@ -18,9 +18,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t by adding poke and peek hooks that
  * monitor writes and reads to the registers implemented within the

host/tests/rfnoc_block_tests/vector_iir_block_test.cpp

@@ -20,9 +20,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 constexpr size_t NUM_CHANS       = 4;
 constexpr uint16_t MAX_DELAY_LEN = 2047;
 constexpr size_t DEFAULT_MTU     = 8000;

host/tests/rfnoc_block_tests/window_block_test.cpp

@@ -18,9 +18,6 @@
 using namespace uhd::rfnoc;
 using namespace uhd::rfnoc::test;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 /*
  * This class extends mock_reg_iface_t by adding poke and peek hooks that
  * monitor writes and reads to the registers implemented within the window

host/tests/rfnoc_block_tests/x4xx_radio_mock.hpp

@@ -12,6 +12,7 @@
 #include <uhd/rfnoc/node_accessor.hpp>
 #include <uhd/utils/log.hpp>
 #include <uhdlib/rfnoc/clock_iface.hpp>
+#include <uhdlib/rfnoc/noc_block_make_args.hpp>
 #include <chrono>
 #include <iostream>
 
@@ -21,9 +22,6 @@ using namespace std::chrono_literals;
 using namespace uhd::usrp::zbx;
 using namespace uhd::experts;
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
-
 namespace {
 /* This class extends mock_reg_iface_t by adding a constructor that initializes
  * some of the read memory to contain the memory size for the radio block.

host/tests/rfnoc_propprop_test.cpp

@@ -12,6 +12,7 @@
 #include <uhd/rfnoc/rfnoc_types.hpp>
 #include <uhd/utils/log.hpp>
 #include <uhdlib/rfnoc/clock_iface.hpp>
+#include <uhdlib/rfnoc/noc_block_make_args.hpp>
 #include <uhdlib/rfnoc/prop_accessor.hpp>
 #include <boost/test/unit_test.hpp>
 #include <algorithm>
@@ -247,17 +248,15 @@ BOOST_AUTO_TEST_CASE(test_failures)
     BOOST_REQUIRE_THROW(node_accessor.resolve_props(&mock2), uhd::resolve_error);
 }
 
-// Redeclare this here, since it's only defined outside of UHD_API
-noc_block_base::make_args_t::~make_args_t() = default;
 
 BOOST_AUTO_TEST_CASE(test_mtu_forwarding_policy_restrictions)
 {
     // Most of this is just dummy stuff required to correctly instantiate a
     // noc_block_base-derived block and is inconsequential to the test itself
     mock_block_container mbc;
-    mbc.reg_iface                     = std::make_shared<mock_reg_iface_t>();
-    mbc.tree                          = uhd::property_tree::make();
-    mbc.make_args                     = std::make_unique<noc_block_base::make_args_t>();
+    mbc.reg_iface = std::make_shared<mock_reg_iface_t>();
+    mbc.tree      = uhd::property_tree::make();
+    mbc.make_args = noc_block_base::make_args_ptr(new noc_block_base::make_args_int_t());
     mbc.make_args->noc_id             = 0x01020304;
     mbc.make_args->block_id           = block_id_t("0/Dummy#0");
     mbc.make_args->num_input_ports    = 2;