mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <bbann...@apache.org>
Subject Re: Review Request 71734: Revamped attribute handling in stout.
Date Wed, 06 Nov 2019 20:16:18 GMT


> On Nov. 6, 2019, 5:03 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Line 65 (original), 65 (patched)
> > <https://reviews.apache.org/r/71734/diff/2/?file=2171707#file2171707line65>
> >
> >     nit: single line?
> 
> Benno Evers wrote:
>     I thought about it, but right now (i.e. after this patch is applied) all code is
inconsistent in using the
>     
>     ```
>         attributes
>         void foo(args);
>     ```
>     style of declaration, which looks pretty good to me.

This is mostly a matter of personal preference, but at least for such simple cases which do
not come with "local consistency" precedence I'd pick whatever our `clang-format` fork would
do, if only to make this less of a hassle in the future (the attribute on a separate line
also has a non-C++ C vibe for me personally). So I guess the only argument I could bring would
be: easier to work with since tooling-enabled.


- Benjamin


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


On Nov. 6, 2019, 4:56 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71734/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2019, 4:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This makes several changes to the attribute compatibility layer
> provided by stout:
> 
>   * Add a new `STOUT_HAS_CPP_ATTRIBUTE` macro to test compiler
>     support for a given attribute.
>   * Renamed the non-namespaced `NORETURN` macro to `STOUT_NORETURN`.
>   * Preferred the use of the standardized `[[noreturn]]` syntax
>     if supported by the compiler.
>   * Fixed previous usages of `NORETURN` in the stout codebase.
>   * Added support for the `[[nodiscard]]` attribute.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/abort.hpp 43ed5ce2830c493e4c801cc81f8dde0922c99a8d 
>   3rdparty/stout/include/stout/attributes.hpp aa377db82e1dbdb8727b1128780e2409accc8ae9

>   3rdparty/stout/include/stout/exit.hpp 34585a005063b17d0c7754c8e8c13f0641383bc4 
>   3rdparty/stout/include/stout/unimplemented.hpp ab6caa8fa9645bca66a3efcdc6d337f3fb0481d7

>   3rdparty/stout/include/stout/unreachable.hpp d4b3bb0582eb9e64e6f150735d1e9f2956edbca6

> 
> 
> Diff: https://reviews.apache.org/r/71734/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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