Messages in this thread | | | From | Nick Desaulniers <> | Date | Thu, 23 Aug 2018 16:12:20 -0700 | Subject | Re: [PATCH] include/linux/compiler*.h: make compiler-*.h mutually exclusive |
| |
On Thu, Aug 23, 2018 at 2:19 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2018-08-23 at 14:03 -0700, Nick Desaulniers wrote: > > One reply for a bunch of the various threads, to keep the number of emails down: > > > > On Wed, Aug 22, 2018 at 5:20 PM Joe Perches <joe@perches.com> wrote: > > > On Wed, 2018-08-22 at 16:37 -0700, Nick Desaulniers wrote: > > > > +/* Compiler specific macros. */ > > > > #ifdef __clang__ > > > > #include <linux/compiler-clang.h> > > > > > > probably better as > > > > > > #if defined(__clang) > > > > > > to match the style of the #elif defined()s below it > > > > Hi Joe, > > Thanks for the feedback. I always appreciate it. If you have some > > cleanups, want to send them to me, and I'll bundle them up for a PR? > > I'm ok with that change. > > > > > > +#ifdef __GNUC_STDC_INLINE__ > > > > +# define __gnu_inline __attribute__((gnu_inline)) > > > > +#else > > > > +# define __gnu_inline > > > > +#endif > > > > > > Perhaps __gnu_inline should be in compiler-gcc and this > > > should use > > > > > > #ifndef __gnu_inline > > > #define __gnu_inline > > > #endif > > > > Not this case; it's how we get gnu89 semantics for `extern inline` is > > not compiler specific (therefor should not go in a compiler specific > > header). > > It's not possible to know that compilers support what > __attribute__((<foo>)) and at what version that support > exists unless it is specified somewhere.
__has_attribute: https://clang.llvm.org/docs/LanguageExtensions.html#has-attribute
The release notes of GCC-5 mention __has_attribute. https://gcc.gnu.org/gcc-5/changes.html
The point of feature detection is that it _doesn't matter_ what version that support exists. Either it does and you can use it, or it doesn't and you can decide whether to stop compiling or there's a valid work around.
Feature detection should be preferred to explicit version checks except in 2 specific cases: 1. It's not possible to properly perform feature detection. Language features should not be added unless it's possible to safely check for them. 2. A very specific version of a very specific compiler is broken and needs to be explicitly guarded against.
> As far as I can tell, gnu_inline is not recognized by clang. > > https://clang.llvm.org/docs/AttributeReference.html
If that was the case, I would not have added it in commit d03db2bc26f0 ("compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations"). https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d03db2bc26f0e4a6849ad649a09c9c73fccdc656
Docs can sometimes fall behind, the lone source of truth is the source code. https://github.com/llvm-mirror/clang/search?q=gnu_inline&unscoped_q=gnu_inline
Godbolt is also incredibly helpful for testing various compiler versions: https://godbolt.org/z/uMJ-mo
https://reviews.llvm.org/D51190
-- Thanks, ~Nick Desaulniers
| |