You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@mesos.apache.org by "Michael Park (JIRA)" <ji...@apache.org> on 2015/06/20 02:25:00 UTC
[jira] [Comment Edited] (MESOS-2664) Modernize the codebase to
C++11
[ https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14594181#comment-14594181 ]
Michael Park edited comment on MESOS-2664 at 6/20/15 12:24 AM:
---------------------------------------------------------------
The only 3rdparty that it affected was {{stout/protobuf.hpp}}, which is also where "spelling out the default" would be the most prevalent due to the number of members in the {{FieldDescriptor}} enum.
With approach (1) we have:
{code}
Try<Nothing> operator () (const JSON::Object& object) const
{
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_MESSAGE:
/* ... */
break;
case google::protobuf::FieldDescriptor::TYPE_DOUBLE:
case google::protobuf::FieldDescriptor::TYPE_FLOAT:
case google::protobuf::FieldDescriptor::TYPE_INT32:
case google::protobuf::FieldDescriptor::TYPE_INT64:
case google::protobuf::FieldDescriptor::TYPE_UINT32:
case google::protobuf::FieldDescriptor::TYPE_UINT64:
case google::protobuf::FieldDescriptor::TYPE_FIXED32:
case google::protobuf::FieldDescriptor::TYPE_FIXED64:
case google::protobuf::FieldDescriptor::TYPE_BOOL:
case google::protobuf::FieldDescriptor::TYPE_STRING:
case google::protobuf::FieldDescriptor::TYPE_GROUP:
case google::protobuf::FieldDescriptor::TYPE_BYTES:
case google::protobuf::FieldDescriptor::TYPE_ENUM:
case google::protobuf::FieldDescriptor::TYPE_SFIXED32:
case google::protobuf::FieldDescriptor::TYPE_SFIXED64:
case google::protobuf::FieldDescriptor::TYPE_SINT32:
case google::protobuf::FieldDescriptor::TYPE_SINT64:
return Error("Not expecting a JSON object for field '" +
field->name() + "'");
}
return Nothing();
}
{code}
With (2) we have:
{code}
Try<Nothing> operator () (const JSON::Object& object) const
{
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wswitch-enum"
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_MESSAGE:
/* ... */
break;
default:
return Error("Not expecting a JSON object for field '" +
field->name() + "'");
}
#pragma GCC diagnostic pop
return Nothing();
}
{code}
(2) is nice in the sense that it's clear {{TYPE_MESSAGE}} is really the only type we'll ever handle. However, considering this is the worst case scenario (having to state 17 cases in place for a {{default}}) that doesn't occur often, I'm also in favor of adopting (1).
was (Author: mcypark):
The only 3rdparty that it affected was {{stout/protobuf.hpp}}, which is also where "spelling out the default" would be the most prevalent due to the number of members in the {{FieldDescriptor}} enum.
With approach (1) we have:
{code}
Try<Nothing> operator () (const JSON::Object& object) const
{
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_MESSAGE:
/* ... */
break;
case google::protobuf::FieldDescriptor::TYPE_DOUBLE:
case google::protobuf::FieldDescriptor::TYPE_FLOAT:
case google::protobuf::FieldDescriptor::TYPE_INT32:
case google::protobuf::FieldDescriptor::TYPE_INT64:
case google::protobuf::FieldDescriptor::TYPE_UINT32:
case google::protobuf::FieldDescriptor::TYPE_UINT64:
case google::protobuf::FieldDescriptor::TYPE_FIXED32:
case google::protobuf::FieldDescriptor::TYPE_FIXED64:
case google::protobuf::FieldDescriptor::TYPE_BOOL:
case google::protobuf::FieldDescriptor::TYPE_STRING:
case google::protobuf::FieldDescriptor::TYPE_GROUP:
case google::protobuf::FieldDescriptor::TYPE_BYTES:
case google::protobuf::FieldDescriptor::TYPE_ENUM:
case google::protobuf::FieldDescriptor::TYPE_SFIXED32:
case google::protobuf::FieldDescriptor::TYPE_SFIXED64:
case google::protobuf::FieldDescriptor::TYPE_SINT32:
case google::protobuf::FieldDescriptor::TYPE_SINT64:
return Error("Not expecting a JSON object for field '" +
field->name() + "'");
}
return Nothing();
}
{code}
With (2) we have:
{code}
Try<Nothing> operator () (const JSON::Object& object) const
{
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wswitch-enum"
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_MESSAGE:
/* ... */
break;
default:
return Error("Not expecting a JSON object for field '" +
field->name() + "'");
}
#pragma GCC diagnostic pop
return Nothing();
}
{code}
(2) is nice in the sense that it's clear {{TYPE_MESSAGE}} is really the only type we'll ever handle. However, considering this is the worst case scenario (having to state 17 cases in place for a {{default}}) that doesn't occur often, I'm in favor of adopting (1).
> Modernize the codebase to C++11
> -------------------------------
>
> Key: MESOS-2664
> URL: https://issues.apache.org/jira/browse/MESOS-2664
> Project: Mesos
> Issue Type: Epic
> Components: technical debt
> Reporter: Michael Park
> Assignee: Michael Park
> Labels: mesosphere
>
> Since [this commit|https://github.com/apache/mesos/commit/0f5c78fad3423181f7227027eb42d162811514e7], we officially require GCC-4.8+ and Clang-3.5+. This means that we now have full C++11 support and therefore can start to modernize our codebase to be more readable, safer and efficient!
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)