Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SWIG %template directives are incorrectly scoped #768

Open
dbankieris opened this issue May 9, 2019 · 5 comments
Open

SWIG %template directives are incorrectly scoped #768

dbankieris opened this issue May 9, 2019 · 5 comments

Comments

@dbankieris
Copy link
Contributor

convert_swig writes a %template directive for each instantiation of a template it finds.

// example.hh

namespace a { 
    template<class> class A {}; 
}

namespace b { 
    class B { 
        public:
        a::A<int> a;
    };
}

The above header file produces a section in the corresponding .i file:

//example.i

namespace a{
    template<class> class A {};
}

namespace b{
    class B {
        public:
        a::A<int> a;
#define TRICK_SWIG_TEMPLATE_Ba_template

    };
    
#ifdef TRICK_SWIG_TEMPLATE_Ba_template
%template (Ba_template) a::A<int>  ;
#undef TRICK_SWIG_TEMPLATE_Ba_template
#endif
#define TRICK_SWIG_DEFINED_b__B
}

According to the SWIG 4.0 documentation:

The %template directive for a class template is the equivalent to an explicit instantiation of a C++ class template. The scope for a valid %template instantiation is the same as the scope required for a valid explicit instantiation of a C++ template. A definition of the template for the explicit instantiation must be in scope where the instantiation is declared and must not be enclosed within a different namespace.

The above example results in the following:

Error: 'a::A' resolves to 'a::A' and was incorrectly instantiated in scope 'b' instead of within scope 'a'.
@dbankieris
Copy link
Contributor Author

dbankieris commented May 10, 2019

I removed the %template-generating code, but that resulted in another issue. According to this post, without an explicit instantiation, SWIG knows nothing about a template's assignment operator. A template with a non-public assignment operator and no %template directive results in a privacy error in the set wrapper code for variables of that type. So we indeed need to write %template directives.

dbankieris added a commit that referenced this issue May 10, 2019
@dbankieris
Copy link
Contributor Author

This problem is silently ignored in versions of SWIG before 4.0, in which it becomes an error. So we have until that becomes the default version to convert convert_swig to clang :)

@alexlin0
Copy link
Contributor

I tested this example with Ubuntu 20.04 with SWIG 4.0. I got different code in the .i file and it compiled successfully.

namespace a{
    template<class> class A {};
}

namespace b{
    class B {
        public:
        a::A<int> a;
#define TRICK_SWIG_TEMPLATE_b__B_a

    };
#define TRICK_SWIG_TEMPLATE_b__B
}

#ifdef TRICK_SWIG_TEMPLATE_b__B_a
%template (b__B_a) a::A<int>  ;
#undef TRICK_SWIG_TEMPLATE_b__B_a
#endif
#ifdef TRICK_SWIG_DEFINED_b__B
%trick_cast_as(b::B, b__B)
#endif

@dbankieris
Copy link
Contributor Author

I added some logic in e6509df to put the %template lines in the global namespace if the type is qualified in any way. It works if the type is fully qualified, but breaks if it's only partially qualified. convert_swig doesn't have namespace information for all the types it might encounter, so that's the best I could come up with. See SIM_swig_template_scoping for examples.

@dbankieris
Copy link
Contributor Author

e6509df worked well enough until SWIG 4.2.0, which requires that a %template directive come before a corresponding template instantiation (see swig/swig#2898). To be fair, this requirement was always in the documentation:

Make sure all necessary `typedef' declarations and type-information is available in the interface file. In particular, ensure that the type information is specified in the correct order as required by a C/C++ compiler. Most importantly, define a type before it is used! A C compiler will tell you if the full type information is not available if it is needed, whereas SWIG will usually not warn or error out as it is designed to work without full type information. However, if type information is not specified correctly, the wrappers can be sub-optimal and even result in uncompilable C/C++ code.

#1679, #1696, and #1741 got things partially working again, but there are still several problems. All of the below is valid C++ code, but convert_swig does not properly convert much of it into valid SWIG input code. While the workaround for most of these is to over-qualify the type, that is not something most users will think to do, and it is contrary to best practice.

template <class T>
class Foo {};

namespace peer {
    template <class T>
    class Bar {};
}

namespace prime {

  template<class T>
  class Potato {};

  namespace nested {
    template <class T>
    class Baz {};
  }

  class Onion {
    public:

      template<class T>
      class InnerOnion {}; // onions have layers!

      /**
       * Error: 'Foo' resolves to '::Foo' and was incorrectly
       * instantiated in scope 'prime' instead of within scope ''.
       */
      //Foo<int> foo1;
      ::Foo<int> foo2;

        peer::Bar<int> bar1;
      ::peer::Bar<int> bar2;

      /**
       * error: ‘Potato’ was not declared in this scope;
       * did you mean ‘prime::Potato’?
       */
            // Potato<int> potato1;
        prime::Potato<int> potato2;
      ::prime::Potato<int> potato3;

      // Error: Undefined scope 'nested'
      //       nested::Baz<int> baz1;
        prime::nested::Baz<int> baz2;
      ::prime::nested::Baz<int> baz3;

      // Error: Template 'InnerOnion' undefined.
      // InnerOnion<int> layer1;

      // Error: Undefined scope 'Onion'
      // Error: Template 'Onion::InnerOnion' undefined.
      // Onion::InnerOnion<int> layer2;

      // Error: Undefined scope 'prime::Onion'
      // Error: Template 'prime::Onion::InnerOnion' undefined.
      //prime::Onion::InnerOnion<int> layer3;

      // Error: Undefined scope 'prime::Onion'
      // Error: Template '::prime::Onion::InnerOnion' undefined.
      //::prime::Onion::InnerOnion<int> layer4;
  };

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants