SWIG 4.3 attempts to wrap friended but not public functions

[luciansmith]

We have the following setup in some of our projects:

foo.h:

class Foo {
   [...]
   friend void randomFunction()
}

foo.cpp:

void randomFunction() {
 [...]
}

Foo::classFunction() {
   randomFunction();
}

With SWIG 4.2, this was fine, and randomFunction was ignored automatically. With 4.3, it notices that randomFunction exists, and tries to wrap it, then gets an error, because it wasn't declared in the #included .h file:

In function ‘PyObject* _wrap_randomFunction():
fooPYTHON_wrap.cxx:44238:21: error: ‘randomFunction ’ was not declared in this scope; did you mean ‘_wrap_randomFunction ’?

The above was created by hand; actual full error at https://github.com/sys-bio/roadrunner/actions/runs/13020366832/job/36319451129

[wsfulton]

SWIG 4.2.1 and 4.3.0 fixed a bug to generate wrappers for friend declarations that previously were not generated. Please see the CHANGES file for details. Handling of friend functions is fully documented, please take a look at https://swig.org/Doc4.3/SWIGPlus.html#SWIGPlus_nn17. You may want to ignore the friend wrappers with %ignore as documented here if you don't want them and want to replicated behaviour of earlier versions of SWIG.

[luciansmith]

I don't understand why SWIG is now designed to fail on purpose? I can indeed %ignore the functions, but 'replicating the behavior of earlier versions of SWIG' in this case means 'it used to work and now it fails'. Can't SWIG be designed to succeed in the first place? In this case, it would be a matter of realizing that the function wasn't declared in any .h file, so deciding to not wrap it.

[wsfulton]

SWIG contains bugs, just like everyone's favourite C/C++ compiler and most other software does. If you don't like the volunteer contributors improving SWIG and fixing bugs as well as providing solutions to replicate previous incorrect behaviour, then please use another product.

I don't understand why SWIG is now designed to fail on purpose?

SWIG is not designed to fail on purpose. SWIG cannot guarantee that whatever one throws at it will work. However, despite the immense complexities and ambiguities in C++ interfaces, SWIG bends over backwards to generate code that compiles by default if it is valid C/C++ and often invalid C++ when type information is missing.

I suggest you familiarise yourself with C++ friends and actually read the documentation I pointed you to and/or familiarise yourself with the C++ specification; friends are actually non-trivial in C++ and I've tried to distill the important aspects with regard to wrappers in the SWIG documentation. If you think SWIG is designed to fail on purpose in this area you could at least provide a full reproducible example instead of snippets of erroneous code that does not compile and provide informed details as to why the documented approach is wrong.

[luciansmith]

I'm not trying to be ungrateful, and I definitely appreciate everything that SWIG's done for us over the last, what, 15-20 years or so that we've been using it? It seemed to me that you closed this issue saying that SWIG was working as intended, when instead it is literally producing uncompilable code. I assumed you are a busy person, and had missed this, or something.

If you now agree that it's a bug, if you want to close the issue with a WONTFIX, that's certainly your prerogative, and there is, as the documentation does indeed point out, a workaround. Fair enough! But it seemed like you closed it with 'NOTABUG' which confused me.

I would be surprised if you needed a test case for this particular issue, as the documentation points it out explicitly, but if you've lost the cases you had at the time and you changed your mind about fixing it, I could try to pull something together. I've been working on monstrous projects in C++ for so long that I've basically lost my ability to do anything simple in it; all my 'simple' energy goes to Python nowadays.

At any rate, sorry for being annoying; I really was just confused.

[wsfulton]

Ah, but I have closed this issue as not a bug and there is nothing to fix in SWIG that I can see from the information you've given. The cause of the new failure in is the result of fixing a bug in SWIG. There is no motivation for SWIG to be bug compatible with older buggy versions of SWIG in this case. While unfortunate that this improvement in SWIG exposes a problem in your project, it is something that will either need fixing or working around on your side now.

[luciansmith]

That's fair, though I do disagree: there's nothing in my project as a C++ project that is incorrect. It compiles; it does what I want. In my opinion, our friending of a non-public function is something that ideally SWIG would notice, and it wouldn't try to wrap a non-public function just because it was separately listed as a friend. (My case is exactly what's called out in the documentation that mentions the error 'error: 'blah' was not declared in this scope'.)

If that's too much work, that's fine! There's the %ignore workaround. But pushing that responsibility to the user feels a little off-brand for my own perception of 'what SWIG does', and is why I would classify it as a bug, or at the very least, a feature request.

[wsfulton]

SWIG does not attempt to go into the long weeds of ADL where some friend declarations/definitions may or may not be visible. You can call it a documented limitation if you like. What you have is a corner case that cannot be dealt with automatically by SWIG; it's somewhat unusual to have randomFunction not taking an instance of Foo in the argument list and this is the cause of your non-visibility.

it wouldn't try to wrap a non-public function just because it was separately listed as a friend.

This is not how friends work. Placing the friend declaration in the private or public section of the class declaration has no effect on the public visibility of the friend function as the friend function is global. Note that just this code:

// main.cxx:
class Foo {
private:
    friend void randomFunction(const Foo& f);
};
int main() {
    Foo f;
    randomFunction(f); 
    return 0;
}

declares a public global function so the simple main will work without any further code in main.cxx. It does of course require an implementation for randomFunction to avoid a linker error, but main does not need to see the declaration nor definition.

You may feel that SWIG could be doing a better job here, and arguably yes, but I disagree this is off-brand. If you want to use SWIG, you must expect to make some customisations to get satisfactory wrappers. SWIG definitely does not deal with all the ambiguities in C/C++ to keep everyone happy and has to make numerous assumptions and where this is not satisfactory the goal is to provide the mechanisms to adjust the wrappers (%ignore in your case). In summary, SWIG is a tool to simplify the process of wrapping a given set of declarations, it does not and can not guarantee every declaration you throw at it will always be satisfactory.

[luciansmith]

OK, fair enough. Thank you for the additional explanation!