Re: Patch for pjsua2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Asterisk Users]     [Asterisk App Development]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [Linux API]
  Powered by Linux