Hi Christian,
I did some testing here and the exception does work great during runtime now!
However, in my opinion, the "throw()" specification is still important
for SWIG. While it's true that throw() is deprecated for C++, it
remains a valid (and important) part of other languages such as Java.
And having throw() in the declaration of the function will help the
programmers during the development process.
For example, if you try to call a function which throws an exception
in Java but forget to put try and catch, the compiler will give an
error such as:
sample.java:155: error: unreported exception Exception; must be caught
or declared to be thrown
But it's now gone, so the developer must now carefully check each
function in order to know whether it can throw an exception.
Best regards,
Ming
On Mon, Oct 15, 2018 at 10:18 AM, Ming <ming@xxxxxxxxx> wrote:
> Hi Christian,
>
> Thank you for the hard work. Let us review and test this. Given the
> scale of the platforms and language binding affected, this may take
> some time. We will update you again.
>
> Best regards,
> Ming
>
>
> On Mon, Oct 15, 2018 at 4:19 AM, Christian Hoff <christian_hoff@xxxxxxx> wrote:
>> Hello all,
>>
>> during the last weeks, I worked hard on the patch for PJSUA2 and C++ 17
>> compatibility again. For those who have forgotten: Currently the PJSUA2
>> headers cannot be compiled with C++ 17 because C++ 17 removes support for
>> exception specifications. But unfortunately PJSUA2 makes have use of
>> exception specifications.
>>
>> The open issue with my last patch was that SWIG no longer generates catch
>> handlers that catch "pj::Error", as Ming pointed out. I have now updated the
>> patch so that SWIG also generates the required catch handlers. Doing so has
>> been a quite difficult, tedious and time-consuming task. But finally I am
>> done and I am happy to share the resulting patch for inclusion into pjsip.
>> The updated patch is attached to this mail.
>>
>> The magic on the SWIG side is done with the "%exception" directive, which
>> installs a default global exception handler. In my updated patch, I am now
>> using this "%exception" directive to generate the catch handler for
>> "pj::Error" (see the new file "pjsip-apps/src/swig/pj_error.i"). The
>> rationale behind this was that I really wanted to remove all the "throw
>> (pj::Error)" from the header files, so I did not want to have a "PJ_THROW"
>> that would expand to nothing during the compile and expand to the old "throw
>> (...)" when using SWIG. "throw (...)" is now deprecated syntax and we should
>> not keep it around for the sake of SWIG in my opinion.
>>
>> Some basic testing was also done:
>>
>> To verify the C++ 17 compatibility, I compiled my application with the
>> installed adopted pjsua2 header files.
>> To verify the generated SWIG language bindings and catch handlers:
>>
>> For the Python language binding, I verified that the compile works.
>> Afterwards I installed the generated language binding and ran the test file
>> "test.py". "test.py" also verifies the exception handling. I also inspected
>> the generated catch handlers for pj::Error visually.
>> For the Java and C# bindings, I verified that the compile works and
>> inspected the generated catch handlers visually. Unfortunately I did not
>> find a suitable unit test for the exception handling, but I am optimistic
>> that it works nevertheless. I could not run the generated code for the C#
>> bindings because I do not have Xamarin installed on my machine. But as I
>> said, at least I can compile the C++ wrapper code for C#.
>>
>> I hope that this patch can now be considered for inclusion into PJSIP.
>>
>>
>> Best regards,
>>
>> Christian
>>
>>
>> On 25/09/18 10:26, Max Truxa wrote:
>>
>> Hello Christian,
>>
>> how about replacing the throw specifications with a macro that expands
>> to nothing when compiling as C++ but `throw(Error)` when being parsed
>> by SWIG?
>>
>> Something like this:
>>
>> #ifdef SWIG // defined if SWIG is parsing the file
>> # define PJ_CAN_THROW throw(Error)
>> #else
>> # define PJ_CAN_THROW
>> #endif
>>
>> struct Foo
>> {
>> void bar() PJ_CAN_THROW;
>> };
>>
>> Regards,
>>
>> - Max
>>
>> On Tue, Sep 25, 2018 at 8:30 AM Christian Hoff <christian_hoff@xxxxxxx>
>> wrote:
>>
>> Hello Ming,
>>
>> this is also an interesting idea. But will this actually work..? I am
>> wondering because SWIG only parses the header files of PJSUA2. However
>> the actual function implementation is in the ".cpp" source files and in
>> these files you can find the code that throws the exceptions and that
>> could use "PJ_THROW()". But to my knowledge SWIG does not parse the
>> source files and would thus not know which functions may call
>> "PJ_THROW". Or am I wrong about this?
>>
>>
>> Best regards,
>>
>> Christian
>>
>>
>> On 25/09/18 03:08, Ming wrote:
>>
>> Hi Christian,
>>
>> We haven't found a good way either. Our idea is to put PJ_THROW()
>> macro to replace throw(), which can somehow be used by SWIG to
>> generate the exception specs.
>>
>> Regards,
>> Ming
>> On Tue, Sep 25, 2018 at 4:21 AM Christian Hoff <christian_hoff@xxxxxxx>
>> wrote:
>>
>> Hello Ming and all,
>>
>> I have now done some research into SWIG and exceptions. After reading
>> the SWIG documentation, I found out that SWIG indeed parses the C++
>> exception specification to figure out for what types of exceptions
>> "catch(..)" should be generated (see
>> http://www.swig.org/Doc3.0/SWIGDocumentation.html#SWIGPlus_exception_specifications
>> ). Exception types that are specified in the exception specification are
>> automatically propagated to the higher language (Java, C# etc.). Now
>> that we need to remove the exception specification, we lose that
>> capability. Thanks to Ming for the hint. However there is the
>> possibility to adapt the SWIG interface file
>> ("pjsip-apps/src/swig/pjsua2.i") and to add explicit "%catches"
>> statements for each method that describe which types of exceptions the
>> method could possibility throw. So I guess I need to rework my patch for
>> this. I just hope that I will find some way to avoid having to annotate
>> every method with "%catches".... I'll see. Or if someone knows how to do
>> it, then I would be very grateful for a reply.
>>
>>
>> Best regards,
>>
>> Christian
>>
>>
>> On 24/09/18 12:43, Christian Hoff wrote:
>>
>> Hello Ming,
>>
>> thanks for your reply! I will have a look into the SWIG topic this
>> evening and will let you know my findings.
>>
>>
>> Best regards,
>>
>> Christian
>>
>>
>> On 24/09/18 09:03, Ming wrote:
>>
>> Hi Christian,
>>
>> We apologize for the delay in response. We're currently reviewing it
>> and have also managed to do some testing. While for C/C++, this seems
>> to be okay, we're still considering how to best handle this for our
>> Android and Java (via SWIG binding), which may still need the "throw"
>> specifications in order for the developers to know that the functions
>> may throw some exceptions.
>>
>> If you have any feedback regarding this, please let us know.
>>
>> Best regards,
>> Ming
>>
>> On Mon, Sep 24, 2018 at 2:32 PM Christian Hoff
>> <christian_hoff@xxxxxxx> wrote:
>>
>> Hello all,
>>
>> some feedback for this patch is still appreciated. In general I am
>> getting somewhat frustrated about the collaboration with people who
>> want
>> to contribute to the project. Already in the past other people have
>> posted patches here to this list, but did not get any response - not
>> even a response what needs to be changed for the patch to be
>> accepted. I
>> think this is not a good mode of collaboration. Everyone who posts a
>> patch here to this mailing list has done this with the intention to
>> help
>> the project PJSIP to move along (and has spent much time developing the
>> patch), so you should at least return the favour and review the patch.
>>
>> This particular issue that I am trying to address with my patch (that
>> the PJSUA2 headers don't compile against C++17) is an issue that you
>> will eventually need to fix anyhow, so why not fix it right now? As of
>> now, modern versions of GCC use C++ 14 by default, it is only a matter
>> of time until C++ 17 will become the default and people will start
>> complaining that there are compile errors from the PJSUA2 headers.
>>
>> Besides this, my own app uses PJSUA2 with C++ 17 features, so I would
>> have to revert back to C++14 if this patch is not accepted upstream.
>> This would be really unnecessary. So I would really appreciate a patch
>> review.
>>
>> Best regards,
>>
>> Christian
>>
>>
>> On 18/09/18 20:57, Christian Hoff wrote:
>>
>> Hello all,
>> Hello Ming & Riza,
>>
>> do you have any feedback regarding this patch for C++ 17 support? I am
>> also willing to rework it if required, but some feedback would be
>> nice.
>>
>>
>> Thanks in advance and kind regards,
>>
>> Christian
>>
>>
>> On 12/09/18 18:23, Christian Hoff wrote:
>>
>> Hello all,
>>
>> currently it is not possible to compile a pjsua2 application with
>> C++17. The reason is that C++17 removes support for dynamic exception
>> specifications (that is "throws ..." declarations at the end of a
>> function). Unfortunately PJSUA2 makes heavy use of exception
>> specifications and most PJSUA2 functions are declared as "throw
>> (Error)" - meaning that they can throw an exception of type "Error"
>> that the programmer is supposed to catch.
>>
>> The consequence is that a lot of compile errors can be observed when
>> compiling a pjsua2 application with C++17. The compile errors look as
>> follows:
>>
>> /usr/local/include/pjsua2/json.hpp:80:33: error: ISO C++17 does not
>> allow dynamic exception specifications
>> virtual string saveString() throw(Error);
>> ^~~~~
>>
>> The compile of the same application worked fine when using the C++14
>> standard, but it got broken when I switched from C++14 to the C++17
>> standard.
>>
>> See
>> https://www.bfilipek.com/2017/05/cpp17-details-fixes-deprecation.html#removing-deprecated-exception-specifications-from-c17
>>
>> and
>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0003r0.html
>> for more background about the removal of exception specifications.
>>
>> To make the application compile again, I had to remove "throw(Error)"
>> in the whole PJSUA2 source code and include files. This seems to be
>> the only viable and good fix for this issue. I wrote my own "sed"
>> script to do that and made some cosmetic corrections afterwards. Then
>> I generated the attached patch file. Could you please apply this
>> patch to the code base or give me feedback what I still need to
>> improve? I verified that pjsip compiles fine with the patch applied.
>>
>> Thank you very much!
>>
>>
>> Best regards,
>>
>> Christian
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
>>
>>
>> _______________________________________________
>> Visit our blog: http://blog.pjsip.org
>>
>> pjsip mailing list
>> pjsip@xxxxxxxxxxxxxxx
>> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>>
_______________________________________________
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
