mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Hindman" <b...@berkeley.edu>
Subject Re: Review Request 33849: mesos: use standard macros for compiler and vendor detection
Date Thu, 04 Jun 2015 14:44:26 GMT


> On May 5, 2015, 4:29 p.m., Cody Maloney wrote:
> > configure.ac, line 575
> > <https://reviews.apache.org/r/33849/diff/1/?file=950367#file950367line575>
> >
> >     Can you check ax_cxx_compiler_version >= Clang/LLVM 3.5 here? Or does that
not work on both OS X and Linux?
> 
> James Peach wrote:
>     Hmm. I don't have a Linux system with clang 3.5 and I'm not quite sure what you are
asking here. I do have a later version of OS X clang and this change works (and automatically
disables unused local typedef warnings).
>     
>     I tried to leave the compiler logic alone in this patch since I don't have systems
to test all the possible combinations. I agree that it would be desirable to separate the
typedef warnings from the compiler versions (though I was confused that GCC and clang seem
to have subtly different names for the same warning). I'm happy to knock up a separate patch
for that if you like.
> 
> James Peach wrote:
>     If you're asking whether we can drop the ``-Wno-unused-local-typedef`` flags on later
clangs, I'm not confident that I have the right set of build dependencies to answer that.
I tested dropping that warning on my version of clang on OS X (later that 3.5) dropping; both
boost and generated protobuf code still spew that warning.
> 
> Cody Maloney wrote:
>     Apple Clang gives a different version string when you give it `clang --version` than
Linux clang
>     
>     Apple:
>     ```
>     Apple LLVM version 6.1.0 (clang-602.0.49) (based on LLVM 3.6.0svn)
>     Target: x86_64-apple-darwin14.3.0
>     Thread model: posix
>     ```
>     
>     Linux:
>     ```
>     clang version 3.6.0 (tags/RELEASE_360/final)
>     Target: x86_64-unknown-linux-gnu
>     Thread model: posix
>     ```
>     
>     
>     For the `-Wno-unused-local-typedef` - that is definitely a needed flag for some of
the Clang + GCC versions we support. It's much more of "Lets try compiling some code which
checks if we'll generate a warning, then if the compiler is clang add `-Wno-unused-local-typedef`,
if it is gcc add `-Wno-unused-local-typedefs` (Note the s).
> 
> James Peach wrote:
>     That sounds like you want to just unconditionally turn ``-Wunused-local-typedef``
off. Either if you test whether it works, then turn it off if it does, then the net effect
is to always turn it off :)
> 
> Cody Maloney wrote:
>     The warning didn't exist in Clang 3.5 though I think, so one of our platforms doesn't
need it. Other than that one compiler+version combo though it is always on
> 
> James Peach wrote:
>     So it should be safe (and desirable) to do ``-Wno-unused-local-typedef`` ``-Wno-unknown-warning-option``
then?
> 
> Cody Maloney wrote:
>     I'd much rather keep around the "unknwon option" warnings if at all possible. There
is a reasonable amount of work I do where I test-enable misc. random warning options to try
to cleanup the codebase some / see if they'd be feasible long-term, and typos do happen.

I dropped this issue for now since this change doesn't remove any functionality only cleans
up how we check for clang versus gcc. James, it would be great if you could follow this patch
up with one that replaces what Cody had to do with something that just checks for clang 3.5
directly and then acts accordingly. Does that make sense?


- Benjamin


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


On May 5, 2015, 5:52 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33849/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2666
>     https://issues.apache.org/jira/browse/MESOS-2666
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Import the AX_COMPILER_VERSION and AX_COMPILER_VENDOR macros from
> the autoconf archive and use them to detect and configure the
> supported copmilers.
> 
> 
> Diffs
> -----
> 
>   configure.ac 14658f6f6920549f1948b8bd8890c67bc067e3dd 
>   m4/ax_compiler_vendor.m4 PRE-CREATION 
>   m4/ax_compiler_version.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33849/diff/
> 
> 
> Testing
> -------
> 
> Make check on CentOS 7 and Mac OS X 10.10.3. Verified that the default CentOS 6 GCC toolchain
is still rejected as too old.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message