Re: Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Till Toenshoff 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/#review135936
---


Ship it!




Ship It!

- Till Toenshoff


On June 2, 2016, 12:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48164/
> ---
> 
> (Updated June 2, 2016, 12:13 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
> check that no direct `nullptr` dereferences are performed (emitted as
> `-Wnull-dereference`). Dereferencing a `nullptr` is undefined
> behavior.
> 
> The bundled googlemock sources contain such an instance when this
> template is instantiated (it is):
> 
> template 
> inline T Invalid() {
>   return const_cast::type&>(
>   *static_cast::type*>(NULL));
> }
> 
> Note that this was fixed upstream with `b5c81098` which was merged
> into googletest `master` on 2014-01-29, but not yet part of a released
> version (of which the last one was on 2013-09-19).
> 
> As mesos is compiled with `-Werror` and we apply the same kinds of
> warning filters to both mesos and googlemock, above code leads to
> mesos being rejected by current clang.
> 
> In this commit we change the way googlemock headers are added from
> `-I` to `-isystem`; the latter of these instructs the compiler to
> assume the added path contains system headers which suppresses all
> warnings originating from the headers found there.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 
> 
> Diff: https://reviews.apache.org/r/48164/diff/
> 
> 
> Testing
> ---
> 
> Tested on a number of Linux configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/
---

(Updated June 2, 2016, 2:13 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Address Till's comment.


Repository: mesos


Description
---

Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
check that no direct `nullptr` dereferences are performed (emitted as
`-Wnull-dereference`). Dereferencing a `nullptr` is undefined
behavior.

The bundled googlemock sources contain such an instance when this
template is instantiated (it is):

template 
inline T Invalid() {
  return const_cast::type&>(
  *static_cast::type*>(NULL));
}

Note that this was fixed upstream with `b5c81098` which was merged
into googletest `master` on 2014-01-29, but not yet part of a released
version (of which the last one was on 2013-09-19).

As mesos is compiled with `-Werror` and we apply the same kinds of
warning filters to both mesos and googlemock, above code leads to
mesos being rejected by current clang.

In this commit we change the way googlemock headers are added from
`-I` to `-isystem`; the latter of these instructs the compiler to
assume the added path contains system headers which suppresses all
warnings originating from the headers found there.


Diffs (updated)
-

  src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 

Diff: https://reviews.apache.org/r/48164/diff/


Testing
---

Tested on a number of Linux configurations in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Benjamin Bannier 


> On June 2, 2016, 11:36 a.m., Till Toenshoff wrote:
> > src/Makefile.am, line 2086
> > 
> >
> > Let's add a comment explaining this move, please.

I added the canonical explanation.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/#review135907
---


On June 2, 2016, 11:31 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48164/
> ---
> 
> (Updated June 2, 2016, 11:31 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
> check that no direct `nullptr` dereferences are performed (emitted as
> `-Wnull-dereference`). Dereferencing a `nullptr` is undefined
> behavior.
> 
> The bundled googlemock sources contain such an instance when this
> template is instantiated (it is):
> 
> template 
> inline T Invalid() {
>   return const_cast::type&>(
>   *static_cast::type*>(NULL));
> }
> 
> Note that this was fixed upstream with `b5c81098` which was merged
> into googletest `master` on 2014-01-29, but not yet part of a released
> version (of which the last one was on 2013-09-19).
> 
> As mesos is compiled with `-Werror` and we apply the same kinds of
> warning filters to both mesos and googlemock, above code leads to
> mesos being rejected by current clang.
> 
> In this commit we change the way googlemock headers are added from
> `-I` to `-isystem`; the latter of these instructs the compiler to
> assume the added path contains system headers which suppresses all
> warnings originating from the headers found there.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 
> 
> Diff: https://reviews.apache.org/r/48164/diff/
> 
> 
> Testing
> ---
> 
> Tested on a number of Linux configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/#review135913
---



Patch looks great!

Reviews applied: [48164]

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 June 2, 2016, 9:31 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48164/
> ---
> 
> (Updated June 2, 2016, 9:31 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
> check that no direct `nullptr` dereferences are performed (emitted as
> `-Wnull-dereference`). Dereferencing a `nullptr` is undefined
> behavior.
> 
> The bundled googlemock sources contain such an instance when this
> template is instantiated (it is):
> 
> template 
> inline T Invalid() {
>   return const_cast::type&>(
>   *static_cast::type*>(NULL));
> }
> 
> Note that this was fixed upstream with `b5c81098` which was merged
> into googletest `master` on 2014-01-29, but not yet part of a released
> version (of which the last one was on 2013-09-19).
> 
> As mesos is compiled with `-Werror` and we apply the same kinds of
> warning filters to both mesos and googlemock, above code leads to
> mesos being rejected by current clang.
> 
> In this commit we change the way googlemock headers are added from
> `-I` to `-isystem`; the latter of these instructs the compiler to
> assume the added path contains system headers which suppresses all
> warnings originating from the headers found there.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 
> 
> Diff: https://reviews.apache.org/r/48164/diff/
> 
> 
> Testing
> ---
> 
> Tested on a number of Linux configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Till Toenshoff 


> On June 2, 2016, 9:36 a.m., Till Toenshoff wrote:
> > Thanks for testing a supplying a neat workaround Benjamin!

s/a supplying/and supplying/


- Till


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/#review135907
---


On June 2, 2016, 9:31 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48164/
> ---
> 
> (Updated June 2, 2016, 9:31 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
> check that no direct `nullptr` dereferences are performed (emitted as
> `-Wnull-dereference`). Dereferencing a `nullptr` is undefined
> behavior.
> 
> The bundled googlemock sources contain such an instance when this
> template is instantiated (it is):
> 
> template 
> inline T Invalid() {
>   return const_cast::type&>(
>   *static_cast::type*>(NULL));
> }
> 
> Note that this was fixed upstream with `b5c81098` which was merged
> into googletest `master` on 2014-01-29, but not yet part of a released
> version (of which the last one was on 2013-09-19).
> 
> As mesos is compiled with `-Werror` and we apply the same kinds of
> warning filters to both mesos and googlemock, above code leads to
> mesos being rejected by current clang.
> 
> In this commit we change the way googlemock headers are added from
> `-I` to `-isystem`; the latter of these instructs the compiler to
> assume the added path contains system headers which suppresses all
> warnings originating from the headers found there.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 
> 
> Diff: https://reviews.apache.org/r/48164/diff/
> 
> 
> Testing
> ---
> 
> Tested on a number of Linux configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Till Toenshoff 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/#review135907
---


Fix it, then Ship it!




Thanks for testing a supplying a neat workaround Benjamin!


src/Makefile.am (line 2086)


Let's add a comment explaining this move, please.


- Till Toenshoff


On June 2, 2016, 9:31 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48164/
> ---
> 
> (Updated June 2, 2016, 9:31 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
> check that no direct `nullptr` dereferences are performed (emitted as
> `-Wnull-dereference`). Dereferencing a `nullptr` is undefined
> behavior.
> 
> The bundled googlemock sources contain such an instance when this
> template is instantiated (it is):
> 
> template 
> inline T Invalid() {
>   return const_cast::type&>(
>   *static_cast::type*>(NULL));
> }
> 
> Note that this was fixed upstream with `b5c81098` which was merged
> into googletest `master` on 2014-01-29, but not yet part of a released
> version (of which the last one was on 2013-09-19).
> 
> As mesos is compiled with `-Werror` and we apply the same kinds of
> warning filters to both mesos and googlemock, above code leads to
> mesos being rejected by current clang.
> 
> In this commit we change the way googlemock headers are added from
> `-I` to `-isystem`; the latter of these instructs the compiler to
> assume the added path contains system headers which suppresses all
> warnings originating from the headers found there.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 
> 
> Diff: https://reviews.apache.org/r/48164/diff/
> 
> 
> Testing
> ---
> 
> Tested on a number of Linux configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 48164: Suppress all warnings inside googlemock.

2016-06-02 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48164/
---

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Current clang-3.9.0 `be8f8b5` will always perform a (mostly) syntactic
check that no direct `nullptr` dereferences are performed (emitted as
`-Wnull-dereference`). Dereferencing a `nullptr` is undefined
behavior.

The bundled googlemock sources contain such an instance when this
template is instantiated (it is):

template 
inline T Invalid() {
  return const_cast::type&>(
  *static_cast::type*>(NULL));
}

Note that this was fixed upstream with `b5c81098` which was merged
into googletest `master` on 2014-01-29, but not yet part of a released
version (of which the last one was on 2013-09-19).

As mesos is compiled with `-Werror` and we apply the same kinds of
warning filters to both mesos and googlemock, above code leads to
mesos being rejected by current clang.

In this commit we change the way googlemock headers are added from
`-I` to `-isystem`; the latter of these instructs the compiler to
assume the added path contains system headers which suppresses all
warnings originating from the headers found there.


Diffs
-

  src/Makefile.am c0be66ab28c452e217fe7c7ead8b1b3c283908cc 

Diff: https://reviews.apache.org/r/48164/diff/


Testing
---

Tested on a number of Linux configurations in internal CI.


Thanks,

Benjamin Bannier