[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[ https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14682159#comment-14682159 ] Benjamin Mahler commented on MESOS-2664: Great, started relying on the compile time error of missed switch cases: https://github.com/apache/mesos/blob/7f352ef886f3116e4bef23b235d87b3182354908/src/common/http.cpp#L53 Also, didn't see any tickets around it but we should start using the 'override' keyword, yeah? Since we've [inherited this from google's style guide|https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance] seems like we just need to do a sweep. 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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[ https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14599114#comment-14599114 ] Adam B commented on MESOS-2664: --- Retargeting this epic to 0.24, since we won't complete all of the issues in time for 0.23. The issues that have been resolved will be listed individually in the CHANGELOG, but we'll have to wait at least one more release before we can claim to have modernized the codebase to C++11. Great work so far though! 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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[
https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14599623#comment-14599623
]
Michael Park commented on MESOS-2664:
-
[~bmahler] I think {{-Wswitch}} _almost_ captures what we want.
Yes, to both (a) and (b). The missing part for {{-Wswitch}} for us is: (c) If
we covered all the cases *and* have a {{default}}, it would be nice for that to
also be a warning. When we add a new enum case, I think what we want is a
warning saying you need to handle this new enum value here! rather than
silently going into the {{default}} case. An example from our codebase:
{code}
switch (volume.mode()) {
case Volume::RW: volumeConfig += :rw; break;
case Volume::RO: volumeConfig += :ro; break;
default:
LOG(FATAL) Unknown Volume mode: volume.mode();
break;
}
{code}
I think we can capture what we want with {{-Wswitch}} + a style guide around
the use {{default}}. What do you think?
P.S. We currently have {{-Wall}} turned on, which includes {{-Wswitch}} :)
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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[
https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14598703#comment-14598703
]
Benjamin Mahler commented on MESOS-2664:
[~mcypark] Hm.. doesn't {{-Wswitch}} capture what we want?
(a) If there's no default, and we missed cases, we get a warning.
(b) For those places in the code where default is handy, we can rely on that
without enumerating all cases.
If you're interested, let's file a ticket under this one to capture the
discussion :)
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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[ https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14594083#comment-14594083 ] Benjamin Mahler commented on MESOS-2664: Not relying on default seems like a nice simplification we could make, happy to move the run-time error to a compile time one :) Curious if it will bleed into our 3rd party dependencies? 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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[
https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14594181#comment-14594181
]
Michael Park commented on MESOS-2664:
-
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}
TryNothing 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}
TryNothing 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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[
https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14592907#comment-14592907
]
Michael Park commented on MESOS-2664:
-
Great idea! We could turn it on as a warning, but I'm not sure how effective
that would be since they're likely going to fly by during {{make}}. (Unless
there's a way to suppress {{make}} enough to clearly see the warnings but not
all the other compile/link commands, etc).
If we were to turn it on as an error, we can either (1) not use {{default}} and
spell out the missing cases explicitly, or (2) use something like {{#pragma GCC
diagnostic ignored -Wswitch-enum}} to temporarily turn it off.
I've put together an experiment
[here|https://github.com/mesosphere/mesos/tree/switch-enum] if you're
interested in taking a look at what it might look like.
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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[ https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14588998#comment-14588998 ] Benjamin Mahler commented on MESOS-2664: Another question, not related to C++11, are we able to enable [Wswitch-enum|https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html] as a warning? Would love to have a compile time warning if we miss any switch cases, rather than crashing in the default case at run-time. If we relied on it, we wouldn't even need the defaults, but that might not be clear to those reading the code. 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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[ https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14569436#comment-14569436 ] Michael Park commented on MESOS-2664: - [~bmahler]: it looks like [~jvanremoortere] created MESOS-2801 to address it. 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 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)
[jira] [Commented] (MESOS-2664) Modernize the codebase to C++11
[
https://issues.apache.org/jira/browse/MESOS-2664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14568136#comment-14568136
]
Benjamin Mahler commented on MESOS-2664:
There doesn't seem to be a ticket for removing the dynamic allocation in
Future? Any reason the {{T*}} inside {{Future::Data}} can't be updated to an
{{OptionT}}?
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
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)
