Make GNUInline consistent with whether we use traditional GNU inline semantics.

[pcc]

Summary

Previously we were setting LangOptions::GNUInline (which controls whether we
use traditional GNU inline semantics) if the language did not have the C99
feature flag set. The trouble with this is that C++ family languages also
do not have that flag set, so we ended up setting this flag in C++ modes
(and working around it in a few places downstream by also checking CPlusPlus).

This also caused us to define the GNUC_GNU_INLINE macro in C++ modes,
wrongly claiming traditional GNU inline semantics in C++ (this also happens
to be what GCC does, but I think it's a GCC bug).

The fix is to check whether the C89 flag is set for the target language,
rather than whether the C99 flag is cleared. This also lets us remove the
CPlusPlus checks.

There is a change in semantics in two other places
where we weren't checking both CPlusPlus and GNUInline
(FunctionDecl::doesDeclarationForceExternallyVisibleDefinition and
FunctionDecl::isInlineDefinitionExternallyVisible), but this change seems to
put us back into line with GCC's semantics (test case: test/CodeGen/inline.c).

While at it, forbid -fgnu89-inline in C++ modes, as GCC doesn't support it,
it didn't have any effect before, and supporting it just makes things more
complicated.

[pcc]

Ping.

[rsmith]

I believe your change to not define __GNUC_GNU_INLINE__ will break recent versions of glibc, which test for the definition of either that or __GNUC_STD_INLINE__ to determine whether __attribute__((__gnu_inline__)) is available: https://sourceware.org/git/?p=glibc.git;a=blob;f=misc/sys/cdefs.h;h=99e94cce869572b724df6b5b8c847cc38a0b3a57;hb=HEAD#l331

[rsmith]

Other than the change to -D__GNUC_GNU_INLINE__, this patch looks good to me. Can we unconditionally predefine that macro in C++ mode?

[pcc]

We always define one of __GNUC_GNU_INLINE__ or __GNUC_STDC_INLINE__ depending on whether GNUInline is set; see lib/Frontend/InitPreprocessor.cpp lines 793-796. So I don't think this would break.

[rsmith]

__GNUC_STDC_INLINE__ means C99 inline semantics, which is just as wrong as __GNUC_GNU_INLINE__ for C++. We have to make a choice between these two to support glibc, and sticking to what g++ does (and what clang previously did) seems like the clear better choice -- the risk of breaking things by making a change here is high, and I don't see that there's any real benefit to changing which lie we tell the user.

[pcc]

That seems like a reasonable position to take. I am aware of some headers which are broken without this change, but it's probably more reasonable to consider this to be a bug in those headers.

[pcc]

Retain the existing logic for deciding whether to define GNUC_GNU_INLINE

[rsmith]

LGTM

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

void CompilerInvocation::setLangDefaults(LangOptions &Opts, InputKind IK,
   Opts.C99 = Std.isC99();
   Opts.C11 = Std.isC11();
   Opts.CPlusPlus = Std.isCPlusPlus();
   Opts.CPlusPlus11 = Std.isCPlusPlus11();
   Opts.CPlusPlus14 = Std.isCPlusPlus14();
   Opts.CPlusPlus1z = Std.isCPlusPlus1z();
   Opts.Digraphs = Std.hasDigraphs();
   Opts.GNUMode = Std.isGNUMode();
-  Opts.GNUInline = !Std.isC99();
+  Opts.GNUInline = Std.isC89();
   Opts.HexFloats = Std.hasHexFloats();
   Opts.ImplicitInt = Std.hasImplicitInt();
 
   // Set OpenCL Version.
   Opts.OpenCL = LangStd == LangStandard::lang_opencl || IK == IK_OpenCL;
   if (LangStd == LangStandard::lang_opencl)
     Opts.OpenCLVersion = 100;
   else if (LangStd == LangStandard::lang_opencl11)
if (Opts.ObjC1) {
     if (Args.hasArg(OPT_fno_objc_infer_related_result_type))
       Opts.ObjCInferRelatedResultType = 0;
     
     if (Args.hasArg(OPT_fobjc_subscripting_legacy_runtime))
       Opts.ObjCSubscriptingLegacyRuntime =
         (Opts.ObjCRuntime.getKind() == ObjCRuntime::FragileMacOSX);
   }
     
-  if (Args.hasArg(OPT_fgnu89_inline))
+  if (Args.hasArg(OPT_fgnu89_inline)) {
+    if (Opts.CPlusPlus)
+      Diags.Report(diag::err_drv_argument_not_allowed_with) << "-fgnu89-inline"
+                                                            << "C++/ObjC++";
+    else
       Opts.GNUInline = 1;
+  }
 
   if (Args.hasArg(OPT_fapple_kext)) {
     if (!Opts.CPlusPlus)
       Diags.Report(diag::warn_c_kext);
     else
       Opts.AppleKext = 1;
   }
 

cfe/trunk/lib/Frontend/InitPreprocessor.cpp

if (const char *Prefix = TI.getUserLabelPrefix())
     Builder.defineMacro("__USER_LABEL_PREFIX__", Prefix);
 
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
     Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else
     Builder.defineMacro("__FINITE_MATH_ONLY__", "0");
 
   if (!LangOpts.MSVCCompat) {
-    if (LangOpts.GNUInline)
+    if (LangOpts.GNUInline || LangOpts.CPlusPlus)
       Builder.defineMacro("__GNUC_GNU_INLINE__");
     else
       Builder.defineMacro("__GNUC_STDC_INLINE__");
 
     // The value written by __atomic_test_and_set.
     // FIXME: This is target-dependent.
     Builder.defineMacro("__GCC_ATOMIC_TEST_AND_SET_TRUEVAL", "1");
 

cfe/trunk/lib/Sema/SemaDecl.cpp

if (RequiresAdjustment) {
     NewQType = Context.getCanonicalType(New->getType());
     NewType = cast<FunctionType>(NewQType);
   }
 
   // If this redeclaration makes the function inline, we may need to add it to
   // UndefinedButUsed.
   if (!Old->isInlined() && New->isInlined() &&
       !New->hasAttr<GNUInlineAttr>() &&
-      (getLangOpts().CPlusPlus || !getLangOpts().GNUInline) &&
+      !getLangOpts().GNUInline &&
       Old->isUsed(false) &&
       !Old->isDefined() && !New->isThisDeclarationADefinition())
     UndefinedButUsed.insert(std::make_pair(Old->getCanonicalDecl(),
                                            SourceLocation()));
 
   // If this redeclaration makes it newly gnu_inline, we don't want to warn
   // about it.
   if (New->hasAttr<GNUInlineAttr>() &&
if (FD) {
 
     // The only way to be included in UndefinedButUsed is if there is an
     // ODR use before the definition. Avoid the expensive map lookup if this
     // is the first declaration.
     if (!FD->isFirstDecl() && FD->getPreviousDecl()->isUsed()) {
       if (!FD->isExternallyVisible())
         UndefinedButUsed.erase(FD);
       else if (FD->isInlined() &&
-               (LangOpts.CPlusPlus || !LangOpts.GNUInline) &&
+               !LangOpts.GNUInline &&
                (!FD->getPreviousDecl()->hasAttr<GNUInlineAttr>()))
         UndefinedButUsed.erase(FD);
     }
 
     // If the function implicitly returns zero (like 'main') or is naked,
     // don't complain about missing return statements.
     if (FD->hasImplicitReturnZero() || FD->hasAttr<NakedAttr>())
       WP.disableCheckFallThrough();

cfe/trunk/lib/Sema/SemaExpr.cpp

if (Func->isImplicitlyInstantiable()) {
     }
   }
 
   // Keep track of used but undefined functions.
   if (!Func->isDefined()) {
     if (mightHaveNonExternalLinkage(Func))
       UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), Loc));
     else if (Func->getMostRecentDecl()->isInlined() &&
-             (LangOpts.CPlusPlus || !LangOpts.GNUInline) &&
+             !LangOpts.GNUInline &&
              !Func->getMostRecentDecl()->hasAttr<GNUInlineAttr>())
       UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), Loc));
   }
 
   // Normally the most current decl is marked used while processing the use and
   // any subsequent decls are marked used by decl merging. This fails with
   // template instantiation since marking can happen at the end of the file
   // and, because of the two phase lookup, this function is called with at

cfe/trunk/test/CodeGen/inline.c

 
 // RUN: echo "C++ tests:"
 // RUN: %clang_cc1 -x c++ %s -triple i386-unknown-unknown -O1 -disable-llvm-optzns -emit-llvm -o - -std=c++98 | FileCheck %s --check-prefix=CHECK3
 // 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 @_Z13gnu_ei_inlinev()
+// CHECK3-NOT: @_Z5testCv
 // CHECK3-LABEL: define linkonce_odr i32 @_Z2eiv()
 
 // RUN: echo "MS C Mode tests:"
 // RUN: %clang_cc1 %s -triple i386-unknown-unknown -O1 -disable-llvm-optzns -emit-llvm -o - -std=c99 -fms-compatibility | FileCheck %s --check-prefix=CHECK4
 // CHECK4-LABEL: define weak_odr i32 @ei()
 // CHECK4-LABEL: define i32 @bar()
 // CHECK4-NOT: unreferenced1
 // CHECK4-LABEL: define weak_odr void @unreferenced2()

cfe/trunk/test/Frontend/gnu-inline.c

+// RUN: %clang_cc1 -std=c89 -fsyntax-only -x c -E -dM %s | FileCheck --check-prefix=GNU-INLINE %s
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -x c -E -dM %s | FileCheck --check-prefix=STDC-INLINE %s
+// RUN: %clang_cc1 -std=c99 -fgnu89-inline -fsyntax-only -x c -E -dM %s | FileCheck --check-prefix=GNU-INLINE %s
+// RUN: %clang_cc1 -fsyntax-only -x c++ -E -dM %s | FileCheck --check-prefix=GNU-INLINE %s
+// RUN: not %clang_cc1 -fgnu89-inline -fsyntax-only -x c++ %s 2>&1 | FileCheck --check-prefix=CXX %s
+
+// CXX: '-fgnu89-inline' not allowed with 'C++/ObjC++'
+
+// STDC-INLINE-NOT: __GNUC_GNU_INLINE__
+// STDC-INLINE: #define __GNUC_STDC_INLINE__ 1
+// STDC-INLINE-NOT: __GNUC_GNU_INLINE__
+
+// GNU-INLINE-NOT: __GNUC_STDC_INLINE__
+// GNU-INLINE: #define __GNUC_GNU_INLINE__ 1
+// GNU-INLINE-NOT: __GNUC_STDC_INLINE__