Skip to content

Commit

Permalink
[clang] [AST] Treat "inline gnu_inline" the same way as "extern inlin…
Browse files Browse the repository at this point in the history
…e gnu_inline" in C++ mode

This matches how GCC handles it, see e.g. https://gcc.godbolt.org/z/HPplnl.
GCC documents the gnu_inline attribute with "In C++, this attribute does
not depend on extern in any way, but it still requires the inline keyword
to enable its special behavior."

The previous behaviour of gnu_inline in C++, without the extern
keyword, can be traced back to the original commit that added
support for gnu_inline, SVN r69045.

Differential Revision: https://reviews.llvm.org/D67414

llvm-svn: 373078
  • Loading branch information
mstorsjo committed Sep 27, 2019
1 parent 5ebab1f commit 71decf8
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 9 deletions.
5 changes: 4 additions & 1 deletion clang/docs/ReleaseNotes.rst
Expand Up @@ -128,7 +128,10 @@ C11 Feature Support
C++ Language Changes in Clang
-----------------------------

- ...
- The behaviour of the `gnu_inline` attribute now matches GCC, for cases
where used without the `extern` keyword. As this is a change compared to
how it behaved in previous Clang versions, a warning is emitted for this
combination.

C++1z Feature Support
^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -3008,6 +3008,10 @@ def warn_gnu_inline_attribute_requires_inline : Warning<
"'gnu_inline' attribute requires function to be marked 'inline',"
" attribute ignored">,
InGroup<IgnoredAttributes>;
def warn_gnu_inline_cplusplus_without_extern : Warning<
"'gnu_inline' attribute without 'extern' in C++ treated as externally"
" available, this changed in Clang 10">,
InGroup<DiagGroup<"gnu-inline-cpp-without-extern">>;
def err_attribute_vecreturn_only_vector_member : Error<
"the vecreturn attribute can only be used on a class or structure with one member, which must be a vector">;
def err_attribute_vecreturn_only_pod_record : Error<
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/AST/Decl.cpp
Expand Up @@ -3261,6 +3261,9 @@ bool FunctionDecl::doesDeclarationForceExternallyVisibleDefinition() const {
return true;
}

if (Context.getLangOpts().CPlusPlus)
return false;

if (Context.getLangOpts().GNUInline || hasAttr<GNUInlineAttr>()) {
// With GNU inlining, a declaration with 'inline' but not 'extern', forces
// an externally visible definition.
Expand Down Expand Up @@ -3289,9 +3292,6 @@ bool FunctionDecl::doesDeclarationForceExternallyVisibleDefinition() const {
return FoundBody;
}

if (Context.getLangOpts().CPlusPlus)
return false;

// C99 6.7.4p6:
// [...] If all of the file scope declarations for a function in a
// translation unit include the inline function specifier without extern,
Expand Down Expand Up @@ -3371,6 +3371,8 @@ bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
// If it's not the case that both 'inline' and 'extern' are
// specified on the definition, then this inline definition is
// externally visible.
if (Context.getLangOpts().CPlusPlus)
return false;
if (!(isInlineSpecified() && getStorageClass() == SC_Extern))
return true;

Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/SemaDeclAttr.cpp
Expand Up @@ -4255,6 +4255,9 @@ static void handleGNUInlineAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
return;
}

if (S.LangOpts.CPlusPlus && Fn->getStorageClass() != SC_Extern)
S.Diag(AL.getLoc(), diag::warn_gnu_inline_cplusplus_without_extern);

D->addAttr(::new (S.Context) GNUInlineAttr(S.Context, AL));
}

Expand Down
3 changes: 2 additions & 1 deletion clang/test/CodeGen/inline.c
Expand Up @@ -52,7 +52,7 @@
// CHECK3-LABEL: define i32 @_Z3barv()
// CHECK3-LABEL: define linkonce_odr i32 @_Z3foov()
// CHECK3-NOT: unreferenced
// CHECK3-LABEL: define void @_Z10gnu_inlinev()
// CHECK3-LABEL: define available_externally void @_Z10gnu_inlinev()
// CHECK3-LABEL: define available_externally void @_Z13gnu_ei_inlinev()
// CHECK3-NOT: @_Z5testCv
// CHECK3-LABEL: define linkonce_odr i32 @_Z2eiv()
Expand Down Expand Up @@ -85,6 +85,7 @@ __inline void unreferenced1() {}
extern __inline void unreferenced2() {}

__inline __attribute((__gnu_inline__)) void gnu_inline() {}
void (*P1)() = gnu_inline;

// PR3988
extern __inline __attribute__((gnu_inline)) void gnu_ei_inline() {}
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCUDA/gnu-inline.cu
Expand Up @@ -7,4 +7,4 @@
// Check that we can handle gnu_inline functions when compiling in CUDA mode.

void foo();
inline __attribute__((gnu_inline)) void bar() { foo(); }
extern inline __attribute__((gnu_inline)) void bar() { foo(); }
9 changes: 9 additions & 0 deletions clang/test/SemaCXX/gnu_inline.cpp
@@ -0,0 +1,9 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s

extern inline
__attribute__((__gnu_inline__))
void gnu_inline1() {}

inline
__attribute__((__gnu_inline__)) // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
void gnu_inline2() {}
6 changes: 3 additions & 3 deletions clang/test/SemaCXX/undefined-inline.cpp
Expand Up @@ -40,20 +40,20 @@ namespace test7 {
}

namespace test8 {
inline void foo() __attribute__((gnu_inline));
inline void foo() __attribute__((gnu_inline)); // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
void test() { foo(); }
}

namespace test9 {
void foo();
void test() { foo(); }
inline void foo() __attribute__((gnu_inline));
inline void foo() __attribute__((gnu_inline)); // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
}

namespace test10 {
inline void foo();
void test() { foo(); }
inline void foo() __attribute__((gnu_inline));
inline void foo() __attribute__((gnu_inline)); // expected-warning {{'gnu_inline' attribute without 'extern' in C++ treated as externally available, this changed in Clang 10}}
}

namespace test11 {
Expand Down

0 comments on commit 71decf8

Please sign in to comment.