nasa / trick

Trick Simulation Environment. Trick provides a common set of simulation capabilities and utilities to build simulations automatically.
Other
34 stars 19 forks source link

Trick doesn't respect deleted copy constructor for class template with array parameter #1731

Closed ninotarantino closed 2 months ago

ninotarantino commented 3 months ago

The Problem

Suppose I have a source file called nino.hh like the following:

/*
PURPOSE: (Testing)

REFERENCE: (())

LIBRARY DEPENDENCY: (())

PROGRAMMER:
   (
    ((Nino Tarantino) (CACI) (05/31/24) (Initial version))
   )
*/
#ifndef NINO_HH
#define NINO_HH

template <class SourceType>
class Nino
{
    public:
        Nino(const SourceType& source)
        :  source(source)
        {
        }

        Nino(const Nino&) = delete;
        Nino& operator=(const Nino&) = delete;

    private:
        const SourceType& source;
};

#endif

and a Trick sim with the following S_define:

#include "sim_objects/default_trick_sys.sm"

##include "nino.hh"
class NinoSimObject : public Trick::SimObject
{
    public:
        NinoSimObject(const NinoSimObject&) = delete;
        NinoSimObject& operator=(const NinoSimObject&) = delete;

        double a[3];
        Nino<double[3]> nino;

        NinoSimObject()
          : nino(a)
        {
        }
};
NinoSimObject nino_sim_object;

I get the build error:

build/home/ataranti/tmp/trick/SIM_template/S_source_py.cpp: In function ‘PyObject* _wrap_NinoSimObject_nino_set(PyObject*, PyObject*)’:
build/home/ataranti/tmp/trick/SIM_template/S_source_py.cpp:16361:29: error: use of deleted function ‘Nino<SourceType>& Nino<SourceType>::operator=(const Nino<SourceType>&) [with SourceType = double [3]]’
   if (arg1) (arg1)->nino = *arg2;
                             ^~~~
In file included from /home/ataranti/tmp/trick/SIM_template/S_source.hh:87,
                 from build/home/ataranti/tmp/trick/SIM_template/S_source_py.cpp:3847:
/home/ataranti/tmp/trick/SIM_template/nino.hh:26:15: note: declared here
         Nino& operator=(const Nino&) = delete;
               ^~~~~~~~
make: *** [build/Makefile_swig:64: build/home/ataranti/tmp/trick/SIM_template/S_source_py.o] Error 1
make: *** Waiting for unfinished jobs....

The sim will build for any other kind of template parameter used for the Nino class: a scalar, pointer, some other class, etc. It's only tripping up on C-style arrays, ex. double[3], int[5], etc.

The Bug

If we take a look at S_source_py.i, that sim object looks like this:

class NinoSimObject : public Trick::SimObject
{
#if SWIG_VERSION > 0x040000
%pythoncode %{
    __setattr__ = _swig_setattr_nondynamic_instance_variable(object.__setattr__)
%}
#endif

    public:
        NinoSimObject(const NinoSimObject&) = delete;
        NinoSimObject& operator=(const NinoSimObject&) = delete;

        double a[3];
        Nino<double[3]> nino;

        NinoSimObject()
          : nino(a)
        {
            Trick::JobData * job __attribute__((unused)) ;
}

    public:
        virtual int call_function( Trick::JobData * curr_job ) ;
        virtual double call_function_double( Trick::JobData * curr_job ) ;

};

We're missing a #define TRICK_SWIG_TEMPLATE_NinoSimObject_nino statement below the nino variable declaration. There should also be lines further down that look like the following, but they're also missing:

#ifdef TRICK_SWIG_TEMPLATE_NinoSimObject_nino
%template(NinoSimObject_nino) Nino<double[3]>;
#undef TRICK_SWIG_TEMPLATE_NinoSimObject_nino
#endif

If I manually add those lines into S_source_py.i the sim will build and run.

Testing

I confirmed this bug using Trick 19.7.1 on:

hchen99 commented 3 months ago

Thanks for reporting this. I believe there was a fix related to this that is not in 19.7.1. Wondering if you tried the latest trick and still saw the issue?

ninotarantino commented 3 months ago

Hey Hong. Just tried with the latest master: c2c068a0 (June 5, 2024) and I'm still seeing the issue.

hchen99 commented 3 months ago

Thanks for your quick response, Nino! Would you give it a try using this: https://github.com/nasa/trick/tree/1703-error-extraneous-endif-and-incorrect-namespaces-after-trick-1679 ? Thanks!

ninotarantino commented 3 months ago

I'm still seeing the issue on that branch too, unfortunately.

hchen99 commented 3 months ago

Thanks for testing. Will investigate.

ninotarantino commented 3 months ago

Thanks for looking into this! For anyone else's future reference, the current work-around is to manually add the SWIG template in source code. For the example I posted, that would be something like

// somesource.hh
#ifdef SWIG
%template(Nino_double3) Nino<double[3]>;
#endif
sharmeye commented 3 months ago

what if you rewrote the template to take, instead of <double[3]>, <double, 3>?

ninotarantino commented 3 months ago

This is for a generic class that can take scalars, pointers, and arrays as template parameters. I could do that, but there'd need to be a decent amount of rewriting to handle arrays since the class as written stores a reference to the base data. I could make the underlying data always be a pointer, but I wanted to avoid that so I wouldn't need to check for null pointers.

Either way, I do have a workaround right now that I'm fine with.

hchen99 commented 3 months ago

@ninotarantino Thanks for sharing the workaround. If you get a chance, please give it a try again using https://github.com/nasa/trick/tree/1703-error-extraneous-endif-and-incorrect-namespaces-after-trick-1679 as a fix for array was put in

ninotarantino commented 2 months ago

No luck unfortunately. I think there's some regex patterns in convert_swig that still need to be updated for it to catch a template of an array type.

hchen99 commented 2 months ago

No luck unfortunately. I think there's some regex patterns in convert_swig that still need to be updated for it to catch a template of an array type.

Thought a specific fix for array type was put in convert_swig. Will double check. Thanks for testing!

hchen99 commented 2 months ago

@ninotarantino Patrick told me that the array type issue will be addressed in a different ticket. Sorry for giving you the incorrect info. Will let you know once the issue is fixed.

hchen99 commented 2 months ago

@ninotarantino https://github.com/nasa/trick/pull/1742 is for addressing array type. Please give it a try whenever you get a chance. Thanks!

ninotarantino commented 2 months ago

@hchen99 Awesome, my example I posted earlier works! I also tried with multidimensional types (ex Type<double[3][3]>) and that works as well. Looks like that fixed the problem.

hchen99 commented 2 months ago

@hchen99 Awesome, my example I posted earlier works! I also tried with multidimensional types (ex Type<double[3][3]>) and that works as well. Looks like that fixed the problem.

Great to hear! Thank you so much for testing and confirming!