You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Aaron Wood via Review Board <no...@reviews.apache.org> on 2017/05/19 18:45:35 UTC
Review Request 59413: Fix constexpr compilation failure with GCC 7.1.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
Diffs
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
src/master/constants.hpp 7edf9f65f
src/sched/constants.hpp 9edb25b38
src/sched/sched.cpp ef73c1dcc
src/scheduler/constants.hpp e3da12646
src/slave/constants.hpp 1159ac3b1
src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
src/slave/slave.cpp 7564e8d39
src/slave/status_update_manager.cpp df63a708c
Diff: https://reviews.apache.org/r/59413/diff/1/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175554
-----------------------------------------------------------
Bad patch!
Reviews applied: [59413]
Failed command: python support/apply-reviews.py -n -r 59413
Error:
error: patch failed: src/slave/constants.hpp:99
error: src/slave/constants.hpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp:45
error: src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp: patch does not apply
Full log: NotAvailableYet\console
- Mesos Reviewbot Windows
On May 19, 2017, 6:45 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 6:45 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 7edf9f65f
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 1159ac3b1
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
> src/slave/slave.cpp 7564e8d39
> src/slave/status_update_manager.cpp df63a708c
>
>
> Diff: https://reviews.apache.org/r/59413/diff/1/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On May 19, 2017, 8:51 p.m., Neil Conway wrote:
> > `stout`, `libprocess`, and `mesos` are nominally distinct projects, so a single RR should touch no more than one of them. i.e., please split the `stout` changes into a separate review.
> >
> > Might also be worth waiting to see if GCC upstream is going to fix the problem promptly. If GCC 7.2 fixes the problem, we don't necessarily need to workaround a bug that occurs in just a single minor release of GCC.
>
> Aaron Wood wrote:
> Sure, I can hold on this and see what they say. If it's something that still needs to be fixed I'll break out this RR into separate ones.
I am not a big fan of the derived classes of `Bytes` since they are practically designed to be used with slicing (`Bytes b = Kilobytes(42)`) which is nasty by itself. I believe the intention here was to provide easy to use factory functions, and don't believe introducing dedicated types for multiples is a very useful but instead confusing (think e.g., number of implicit conversions and overload resolution). This fix makes sense regardless of whether GCC fixes this particular regression.
> On May 19, 2017, 8:51 p.m., Neil Conway wrote:
> > src/master/constants.hpp
> > Line 49 (original), 49 (patched)
> > <https://reviews.apache.org/r/59413/diff/1/?file=1725380#file1725380line49>
> >
> > This change (and all the similar changes) seems unfortunate. Can't we play a similar trick to the change you made to `Bytes`?
>
> Aaron Wood wrote:
> I had wanted to but I didn't see a quick and easy way to do this without making some major changes. The extended classes in `duration.hpp` mostly look something like this:
> ```
> class Nanoseconds : public Duration
> {
> public:
> explicit constexpr Nanoseconds(int64_t nanoseconds)
> : Duration(nanoseconds, NANOSECONDS) {}
>
> constexpr Nanoseconds(const Duration& d) : Duration(d) {}
>
> double value() const { return static_cast<double>(this->ns()); }
>
> static std::string units() { return "ns"; }
> };
> ```
> I'm open to ideas, I just figured if I changed these clases a lot I'd have even more code to change around Mesos.
It looks like the only custom method here is `units` which seems to be used by just the lengthy stringification function below. I believe getting rid of the derived classes here makes just as much sense as it did for `Bytes` and friends.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175555
-----------------------------------------------------------
On May 19, 2017, 9 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 9 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 725680b1e
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 9c1d7245c
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7
> src/slave/slave.cpp 14de72fa4
> src/slave/status_update_manager.cpp 0cd88ac3a
>
>
> Diff: https://reviews.apache.org/r/59413/diff/3/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > `stout`, `libprocess`, and `mesos` are nominally distinct projects, so a single RR should touch no more than one of them. i.e., please split the `stout` changes into a separate review.
> >
> > Might also be worth waiting to see if GCC upstream is going to fix the problem promptly. If GCC 7.2 fixes the problem, we don't necessarily need to workaround a bug that occurs in just a single minor release of GCC.
>
> Aaron Wood wrote:
> Sure, I can hold on this and see what they say. If it's something that still needs to be fixed I'll break out this RR into separate ones.
>
> Benjamin Bannier wrote:
> I am not a big fan of the derived classes of `Bytes` since they are practically designed to be used with slicing (`Bytes b = Kilobytes(42)`) which is nasty by itself. I believe the intention here was to provide easy to use factory functions, and don't believe introducing dedicated types for multiples is a very useful but instead confusing (think e.g., number of implicit conversions and overload resolution). This fix makes sense regardless of whether GCC fixes this particular regression.
I think it is much cleaner as well. If everyone is in agreement I can move forward with this patch regardless of the GCC issue.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > src/master/constants.hpp
> > Line 49 (original), 49 (patched)
> > <https://reviews.apache.org/r/59413/diff/1/?file=1725380#file1725380line49>
> >
> > This change (and all the similar changes) seems unfortunate. Can't we play a similar trick to the change you made to `Bytes`?
>
> Aaron Wood wrote:
> I had wanted to but I didn't see a quick and easy way to do this without making some major changes. The extended classes in `duration.hpp` mostly look something like this:
> ```
> class Nanoseconds : public Duration
> {
> public:
> explicit constexpr Nanoseconds(int64_t nanoseconds)
> : Duration(nanoseconds, NANOSECONDS) {}
>
> constexpr Nanoseconds(const Duration& d) : Duration(d) {}
>
> double value() const { return static_cast<double>(this->ns()); }
>
> static std::string units() { return "ns"; }
> };
> ```
> I'm open to ideas, I just figured if I changed these clases a lot I'd have even more code to change around Mesos.
>
> Benjamin Bannier wrote:
> It looks like the only custom method here is `units` which seems to be used by just the lengthy stringification function below. I believe getting rid of the derived classes here makes just as much sense as it did for `Bytes` and friends.
That's good to know, I didn't dig too deeply to see what was using some of this stuff. I can rework this file a bit so that we could keep the `Duration` base type everywhere like it was.
- Aaron
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175555
-----------------------------------------------------------
On May 19, 2017, 7 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 7 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 725680b1e
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 9c1d7245c
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7
> src/slave/slave.cpp 14de72fa4
> src/slave/status_update_manager.cpp 0cd88ac3a
>
>
> Diff: https://reviews.apache.org/r/59413/diff/3/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > `stout`, `libprocess`, and `mesos` are nominally distinct projects, so a single RR should touch no more than one of them. i.e., please split the `stout` changes into a separate review.
> >
> > Might also be worth waiting to see if GCC upstream is going to fix the problem promptly. If GCC 7.2 fixes the problem, we don't necessarily need to workaround a bug that occurs in just a single minor release of GCC.
Sure, I can hold on this and see what they say. If it's something that still needs to be fixed I'll break out this RR into separate ones.
> On May 19, 2017, 6:51 p.m., Neil Conway wrote:
> > src/master/constants.hpp
> > Line 49 (original), 49 (patched)
> > <https://reviews.apache.org/r/59413/diff/1/?file=1725380#file1725380line49>
> >
> > This change (and all the similar changes) seems unfortunate. Can't we play a similar trick to the change you made to `Bytes`?
I had wanted to but I didn't see a quick and easy way to do this without making some major changes. The extended classes in `duration.hpp` mostly look something like this:
```
class Nanoseconds : public Duration
{
public:
explicit constexpr Nanoseconds(int64_t nanoseconds)
: Duration(nanoseconds, NANOSECONDS) {}
constexpr Nanoseconds(const Duration& d) : Duration(d) {}
double value() const { return static_cast<double>(this->ns()); }
static std::string units() { return "ns"; }
};
```
I'm open to ideas, I just figured if I changed these clases a lot I'd have even more code to change around Mesos.
- Aaron
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175555
-----------------------------------------------------------
On May 19, 2017, 6:51 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 6:51 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 7edf9f65f
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 1159ac3b1
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
> src/slave/slave.cpp 7564e8d39
> src/slave/status_update_manager.cpp df63a708c
>
>
> Diff: https://reviews.apache.org/r/59413/diff/2/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175555
-----------------------------------------------------------
`stout`, `libprocess`, and `mesos` are nominally distinct projects, so a single RR should touch no more than one of them. i.e., please split the `stout` changes into a separate review.
Might also be worth waiting to see if GCC upstream is going to fix the problem promptly. If GCC 7.2 fixes the problem, we don't necessarily need to workaround a bug that occurs in just a single minor release of GCC.
src/master/constants.hpp
Line 49 (original), 49 (patched)
<https://reviews.apache.org/r/59413/#comment248949>
This change (and all the similar changes) seems unfortunate. Can't we play a similar trick to the change you made to `Bytes`?
- Neil Conway
On May 19, 2017, 6:45 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 6:45 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 7edf9f65f
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 1159ac3b1
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
> src/slave/slave.cpp 7564e8d39
> src/slave/status_update_manager.cpp df63a708c
>
>
> Diff: https://reviews.apache.org/r/59413/diff/1/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175556
-----------------------------------------------------------
Bad patch!
Reviews applied: [59413]
Failed command: python support/apply-reviews.py -n -r 59413
Error:
error: patch failed: src/slave/constants.hpp:99
error: src/slave/constants.hpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp:45
error: src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp: patch does not apply
Full log: NotAvailableYet\console
- Mesos Reviewbot Windows
On May 19, 2017, 6:51 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 6:51 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 7edf9f65f
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 1159ac3b1
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
> src/slave/slave.cpp 7564e8d39
> src/slave/status_update_manager.cpp df63a708c
>
>
> Diff: https://reviews.apache.org/r/59413/diff/2/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review176027
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/stout/include/stout/bytes.hpp
Lines 125 (patched)
<https://reviews.apache.org/r/59413/#comment249362>
Unfortunately these definititons break the line length limit. You'll have to do:
```
inline constexpr Bytes Kilobytes(uint64_t value)
{
return Bytes(value, Bytes::KILOBYTES);
}
```
- James Peach
On May 22, 2017, 4:56 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 22, 2017, 4:56 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
>
>
> Diff: https://reviews.apache.org/r/59413/diff/4/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review178318
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/stout/include/stout/bytes.hpp
Line 122 (original), 121 (patched)
<https://reviews.apache.org/r/59413/#comment252201>
2 lines apart.
- Jie Yu
On May 25, 2017, 6:13 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 6:13 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
>
>
> Diff: https://reviews.apache.org/r/59413/diff/5/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc) && make check -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review176162
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59454, 59413]
Passed command: support\windows-build.bat
- Mesos Reviewbot Windows
On May 25, 2017, 6:13 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 6:13 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
>
>
> Diff: https://reviews.apache.org/r/59413/diff/5/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc) && make check -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review176159
-----------------------------------------------------------
Ship it!
Ship It!
- James Peach
On May 25, 2017, 6:13 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 6:13 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
>
>
> Diff: https://reviews.apache.org/r/59413/diff/5/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc) && make check -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review176178
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59454, 59413]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On May 25, 2017, 11:13 a.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 11:13 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
>
>
> Diff: https://reviews.apache.org/r/59413/diff/5/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc) && make check -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 25, 2017, 6:13 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
Diffs
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
Diff: https://reviews.apache.org/r/59413/diff/5/
Testing (updated)
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc) && make check -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 25, 2017, 4:42 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Changes
-------
Fixed formatting.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
Diffs (updated)
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
Diff: https://reviews.apache.org/r/59413/diff/5/
Changes: https://reviews.apache.org/r/59413/diff/4-5/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 22, 2017, 4:56 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
Diffs
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
Diff: https://reviews.apache.org/r/59413/diff/4/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure
with GCC 7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 22, 2017, 4:52 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Changes
-------
Rework patch to only target bytes.hpp.
Summary (updated)
-----------------
Fix bytes.hpp constexpr compilation failure with GCC 7.1.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description (updated)
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`.
Diffs (updated)
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
Diff: https://reviews.apache.org/r/59413/diff/4/
Changes: https://reviews.apache.org/r/59413/diff/3-4/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/#review175651
-----------------------------------------------------------
Can you please split this into 2 separate patches for `Bytes` and `Duration`?
- James Peach
On May 19, 2017, 7 p.m., Aaron Wood wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> -----------------------------------------------------------
>
> (Updated May 19, 2017, 7 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
>
>
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
>
> This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/bytes.hpp 98223db50
> src/master/constants.hpp 725680b1e
> src/sched/constants.hpp 9edb25b38
> src/sched/sched.cpp ef73c1dcc
> src/scheduler/constants.hpp e3da12646
> src/slave/constants.hpp 9c1d7245c
> src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7
> src/slave/slave.cpp 14de72fa4
> src/slave/status_update_manager.cpp 0cd88ac3a
>
>
> Diff: https://reviews.apache.org/r/59413/diff/3/
>
>
> Testing
> -------
>
> `./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
>
>
> Thanks,
>
> Aaron Wood
>
>
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 19, 2017, 7 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Changes
-------
Reference filed GCC bug.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description (updated)
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
Diffs
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
src/master/constants.hpp 725680b1e
src/sched/constants.hpp 9edb25b38
src/sched/sched.cpp ef73c1dcc
src/scheduler/constants.hpp e3da12646
src/slave/constants.hpp 9c1d7245c
src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7
src/slave/slave.cpp 14de72fa4
src/slave/status_update_manager.cpp 0cd88ac3a
Diff: https://reviews.apache.org/r/59413/diff/3/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 19, 2017, 6:58 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Changes
-------
Clean base for patch to be applied.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
Diffs (updated)
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
src/master/constants.hpp 725680b1e
src/sched/constants.hpp 9edb25b38
src/sched/sched.cpp ef73c1dcc
src/scheduler/constants.hpp e3da12646
src/slave/constants.hpp 9c1d7245c
src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 929b7e0b7
src/slave/slave.cpp 14de72fa4
src/slave/status_update_manager.cpp 0cd88ac3a
Diff: https://reviews.apache.org/r/59413/diff/3/
Changes: https://reviews.apache.org/r/59413/diff/2-3/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood
Re: Review Request 59413: Fix constexpr compilation failure with GCC
7.1.
Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59413/
-----------------------------------------------------------
(Updated May 19, 2017, 6:51 p.m.)
Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil Conway.
Changes
-------
Rebased on the latest HEAD.
Bugs: MESOS-7520
https://issues.apache.org/jira/browse/MESOS-7520
Repository: mesos
Description
-------
Many of the `constexpr` variables fail to compile with errors such as `error: 'Megabytes(32).Megabytes::<anonymous>' is not a constant expression because it refers to an incompletely initialized variable`.
This patch changes around the `Bytes` class a bit by getting rid of the small classes that extend `Bytes`. Additionally, any usage of the `Duration` class has been adjusted to not instantiate using the base type (which triggers the issue along with `constexpr`).
Diffs (updated)
-----
3rdparty/stout/include/stout/bytes.hpp 98223db50
src/master/constants.hpp 7edf9f65f
src/sched/constants.hpp 9edb25b38
src/sched/sched.cpp ef73c1dcc
src/scheduler/constants.hpp e3da12646
src/slave/constants.hpp 1159ac3b1
src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp cf0466c62
src/slave/slave.cpp 7564e8d39
src/slave/status_update_manager.cpp df63a708c
Diff: https://reviews.apache.org/r/59413/diff/2/
Changes: https://reviews.apache.org/r/59413/diff/1-2/
Testing
-------
`./bootstrap && mkdir build && cd build && ../configure --disable-python --disable-java --disable-optimize --disable-hardening && make -j$(nproc)`
Thanks,
Aaron Wood