mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Should we remove `noexcept` from `ObjectApprover::approved()` signature?
Date Thu, 14 Jun 2018 11:12:17 GMT
Hi,

I still believe that declaring methods of this module interface `except` is a good thing which
IMO we should also do for all new module interfaces going forward. We do not perform any exception
handling around calls to these functions in Mesos, and `noexcept` is intended to communicate
exactly that.  


Googletest (which has in the meantime absorbed googlemock) is preparing their 1.9.0 release
with C++11 support, and it is expected to be released “soon”, so I am not sure that breaking
backwards compatibility to end up with weaker interfaces will be that worthwhile in the long
run. I left a sketch for a possible workaround in the issue you created, https://issues.apache.org/jira/browse/MESOS-8991?focusedCommentId=16512313.

Let’s continue this discussion on the dev mailing list.


Cheers,


Benjamin


> On Jun 14, 2018, at 12:47 PM, Benno Evers <bevers@mesosphere.com> wrote:
> 
> Hi Alexander,
> 
> > and it is compiled without exception support by default.
> 
> What exactly do you mean by "without support"? My local libmesos.so includes 500 KiB
of unwind tables, and we had issues like MESOS-8417 that are caused by unexpected exceptions
being thrown.
> 
> On Thu, Jun 14, 2018 at 12:10 PM, Alexander Rojas <alexander@mesosphere.io> wrote:
> I may have brought up this issue in the past, however I’m bringing it again, The `ObjectApprover::approved()`
[1] method has the following signature:
> 
> ```
> virtual Try<bool> approved(
>       const Option<Object>& object) const noexcept = 0;
> ```
> 
> This is unfortunate since it is impossible to mock a function in google mock with two
qualifiers [2] without some modifications to gmock itself. this reduces the amount of tests
we can perform.
> 
> Moreover, the `noexcept` qualifier is not even needed in Mesos, since it does not use
exceptions and it is compiled without exception support by default.
> 
> The tricky situation here is that this is a public API so it would be tricky to replace
since it will break backwards compatibility. So I’m calling out to any modules developer
to notify if they are ok with the change or if we should instead try to modify gmock.
> 
> [1] https://github.com/apache/mesos/blob/8b93fa3/include/mesos/authorizer/authorizer.hpp#L221
> [2] https://groups.google.com/forum/#!topic/googlemock/LsbubY26qx4
> 
> 
> 
> Alexander Rojas
> alexander@mesosphere.io
> 
> 
> 
> 
> 
> 
> 
> -- 
> Benno Evers
> Software Engineer, Mesosphere


Mime
View raw message