Disabling of "redefine_extname" pragma for C++ code

[andreybokhanko]

Summary

In response to Richard Smith's comment (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131782.html), this patch disables "redefine_extname" pragma for C++ code. Also, I added a test that this pragma doesn't apply to static declarations.

[andreybokhanko]

Ping!

[andreybokhanko]

Patch updated in response to Aaron's comments (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150706/132411.html).

[andreybokhanko]

Patch updated in response to Aaron's comments (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150706/132552.html).

[rsmith]

Just a couple of minor things, otherwise this looks good to me.

lib/Sema/SemaDecl.cpp

5566–5567

Drop the else here.

test/CodeGen/redefine_extname.c

29

This expected-warning does nothing, because you don't have a -verify RUN: line. This part of the test should be in test/Sema/something.

test/CodeGenCXX/redefine_extname.cpp

34

Likewise.

[andreybokhanko]

Fix Richard Smith's notes:

1. "else" removed form isDeclExternC
2. Two new Sema tests added: test/Sema/redefine_extname.c, test/SemaCXX/redefine_extname.cpp

[andreybokhanko]

All Richard Smith's notes are fixed.

[andreybokhanko]

Ping!

[andreybokhanko]

Added "const" specifier as recommended by Aaron (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150713/133400.html).

include/clang/Basic/DiagnosticSemaKinds.td

 def warn_zero_size_struct_union_in_extern_c : Warning<"%select{|empty }0"
   "%select{struct|union}1 has size 0 in C, %select{size 1|non-zero size}2 in C++">,
   InGroup<ExternCCompat>;
 def warn_cast_qual : Warning<"cast from %0 to %1 drops %select{const and "
   "volatile qualifiers|const qualifier|volatile qualifier}2">,
   InGroup<CastQual>, DefaultIgnore;
 def warn_cast_qual2 : Warning<"cast from %0 to %1 must have all intermediate "
   "pointers const qualified to be safe">, InGroup<CastQual>, DefaultIgnore;
+def warn_redefine_extname_not_applied : Warning<
+  "#pragma redefine_extname is applicable to external C declarations only; "
+  "not applied to %select{function|variable}0 %1">,
+  InGroup<Pragmas>;
 } // End of general sema category.
 
 // inline asm.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_lvalue_in_output : Error<"invalid lvalue in asm output">;
   def err_asm_invalid_output_constraint : Error<
     "invalid output constraint '%0' in asm">;
   def err_asm_invalid_lvalue_in_input : Error<

lib/Sema/SemaDecl.cpp

bool Sema::adjustContextForLocalExternDecl(DeclContext *&DC) {
   //
   // Per C++11 [namespace.def]p6, the innermost enclosing namespace is a
   // semantically-enclosing namespace, not a lexically-enclosing one.
   while (!DC->isFileContext() && !isa<LinkageSpecDecl>(DC))
     DC = DC->getParent();
   return true;
 }
 
-/// \brief Returns true if given declaration is TU-scoped and externally
-/// visible.
-static bool isDeclTUScopedExternallyVisible(const Decl *D) {
+/// \brief Returns true if given declaration has external C language linkage.
+static bool isDeclExternC(const Decl *D) {
   if (auto *FD = dyn_cast<FunctionDecl>(D))
-    return (FD->getDeclContext()->isTranslationUnit() || FD->isExternC()) &&
-           FD->hasExternalFormalLinkage();
-  else if (auto *VD = dyn_cast<VarDecl>(D))
-    return (VD->getDeclContext()->isTranslationUnit() || VD->isExternC()) &&
-           VD->hasExternalFormalLinkage();
+    return FD->isExternC();
+  if (auto *VD = dyn_cast<VarDecl>(D))
+    return VD->isExternC();
 
   llvm_unreachable("Unknown type of decl!");
 }
 
 NamedDecl *
 Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
                               TypeSourceInfo *TInfo, LookupResult &Previous,
                               MultiTemplateParamsArg TemplateParamLists,
if (S->getFnParent() != nullptr) {
       if (!R->isIntegralType(Context) && !R->isPointerType()) {
         Diag(D.getLocStart(), diag::err_asm_bad_register_type);
         NewVD->setInvalidDecl(true);
       }
     }
 
     NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0),
                                                 Context, Label, 0));
-  } else if (!ExtnameUndeclaredIdentifiers.empty() &&
-             isDeclTUScopedExternallyVisible(NewVD)) {
+  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
     llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
       ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
     if (I != ExtnameUndeclaredIdentifiers.end()) {
+      if (isDeclExternC(NewVD)) {
         NewVD->addAttr(I->second);
         ExtnameUndeclaredIdentifiers.erase(I);
+      } else
+        Diag(NewVD->getLocation(), diag::warn_redefine_extname_not_applied)
+            << /*Variable*/1 << NewVD;
     }
   }
 
   // Diagnose shadowed variables before filtering for scope.
   if (D.getCXXScopeSpec().isEmpty())
     CheckShadow(S, NewVD, Previous);
 
   // Don't consider existing declarations that are in a different
FilterLookupForScope(Previous, OriginalDC, S, shouldConsiderLinkage(NewFD),
                        isFunctionTemplateSpecialization);
 
   // Handle GNU asm-label extension (encoded as an attribute).
   if (Expr *E = (Expr*) D.getAsmLabel()) {
     // The parser guarantees this is a string.
     StringLiteral *SE = cast<StringLiteral>(E);
     NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0), Context,
                                                 SE->getString(), 0));
-  } else if (!ExtnameUndeclaredIdentifiers.empty() &&
-             isDeclTUScopedExternallyVisible(NewFD)) {
+  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
     llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
       ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
     if (I != ExtnameUndeclaredIdentifiers.end()) {
+      if (isDeclExternC(NewFD)) {
         NewFD->addAttr(I->second);
         ExtnameUndeclaredIdentifiers.erase(I);
+      } else
+        Diag(NewFD->getLocation(), diag::warn_redefine_extname_not_applied)
+            << /*Variable*/0 << NewFD;
     }
   }
 
   // Copy the parameter declarations from the declarator D to the function
   // declaration NewFD, if they are available.  First scavenge them into Params.
   SmallVector<ParmVarDecl*, 16> Params;
   if (D.isFunctionDeclarator()) {
     DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
NamedDecl *PrevDecl = LookupSingleName(TUScope, Name, NameLoc,
                                          LookupOrdinaryName);
   AsmLabelAttr *Attr =
       AsmLabelAttr::CreateImplicit(Context, AliasName->getName(), AliasNameLoc);
 
   // If a declaration that:
   // 1) declares a function or a variable
   // 2) has external linkage
   // already exists, add a label attribute to it.
-  if (PrevDecl &&
-      (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl)) &&
-      PrevDecl->hasExternalFormalLinkage())
+  if (PrevDecl && (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl))) {
+    if (isDeclExternC(PrevDecl))
       PrevDecl->addAttr(Attr);
-  // Otherwise, add a label atttibute to ExtnameUndeclaredIdentifiers.
     else
+      Diag(PrevDecl->getLocation(), diag::warn_redefine_extname_not_applied)
+          << /*Variable*/(isa<FunctionDecl>(PrevDecl) ? 0 : 1) << PrevDecl;
+  // Otherwise, add a label atttibute to ExtnameUndeclaredIdentifiers.
+  } else
     (void)ExtnameUndeclaredIdentifiers.insert(std::make_pair(Name, Attr));
 }
 
 void Sema::ActOnPragmaWeakID(IdentifierInfo* Name,
                              SourceLocation PragmaLoc,
                              SourceLocation NameLoc) {
   Decl *PrevDecl = LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName);
 

test/CodeGen/redefine_extname.c

 #pragma redefine_extname foo bar
 int f() {
   int foo = 0;
   return foo;
 }
 extern int foo() { return 1; }
 // CHECK: define i32 @bar()
 
+// Check that pragma redefine_extname applies to external declarations only.
+#pragma redefine_extname foo_static bar_static
+static int foo_static() { return 1; }
+int baz() { return foo_static(); }
+// CHECK-NOT: call i32 @bar_static()
+

test/CodeGenCXX/redefine_extname.cpp

int f() {
   int foo = 0;
   return foo;
 }
 extern "C" {
   int foo() { return 1; }
 // CHECK: define i32 @bar()
 }
 
+// Check that pragma redefine_extname applies to C code only, and shouldn't be
+// applied to C++.
+#pragma redefine_extname foo_cpp bar_cpp
+extern int foo_cpp() { return 1; }
+// CHECK-NOT: define i32 @bar_cpp()
+

test/Sema/redefine_extname.c

+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -Wpragmas -verify %s
+
+// Check that pragma redefine_extname applies to external declarations only.
+#pragma redefine_extname foo_static bar_static
+static int foo_static() { return 1; } // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_static'}}
+

test/SemaCXX/redefine_extname.cpp

+// RUN: %clang_cc1 -triple=x86_64-unknown-linux -Wpragmas -verify %s
+
+// Check that pragma redefine_extname applies to C code only, and shouldn't be
+// applied to C++.
+#pragma redefine_extname foo_cpp bar_cpp
+extern int foo_cpp() { return 1; } // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_cpp'}}