r184223 - DR14, DR101, and part of DR1: fix handling of extern "C" declarations in

Richard Smith richard-llvm at metafoo.co.uk
Tue Jun 18 13:15:12 PDT 2013


Author: rsmith
Date: Tue Jun 18 15:15:12 2013
New Revision: 184223

URL: http://llvm.org/viewvc/llvm-project?rev=184223&view=rev
Log:
DR14, DR101, and part of DR1: fix handling of extern "C" declarations in
namespaces, by treating them just like we treat extern "C" declarations in
function scope.

Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/test/CXX/drs/dr0xx.cpp
    cfe/trunk/test/CXX/drs/dr1xx.cpp
    cfe/trunk/test/SemaCXX/PR10447.cpp
    cfe/trunk/test/SemaCXX/extern-c.cpp
    cfe/trunk/www/cxx_dr_status.html

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Jun 18 15:15:12 2013
@@ -341,8 +341,7 @@ public:
   llvm::DenseMap<DeclarationName, NamedDecl *> LocallyScopedExternCDecls;
 
   /// \brief Look for a locally scoped extern "C" declaration by the given name.
-  llvm::DenseMap<DeclarationName, NamedDecl *>::iterator
-  findLocallyScopedExternCDecl(DeclarationName Name);
+  NamedDecl *findLocallyScopedExternCDecl(DeclarationName Name);
 
   typedef LazyVector<VarDecl *, ExternalSemaSource,
                      &ExternalSemaSource::ReadTentativeDefinitions, 2, 2>
@@ -1408,9 +1407,7 @@ public:
 
   NamedDecl *HandleDeclarator(Scope *S, Declarator &D,
                               MultiTemplateParamsArg TemplateParameterLists);
-  void RegisterLocallyScopedExternCDecl(NamedDecl *ND,
-                                        const LookupResult &Previous,
-                                        Scope *S);
+  void RegisterLocallyScopedExternCDecl(NamedDecl *ND, Scope *S);
   bool DiagnoseClassNameShadow(DeclContext *DC, DeclarationNameInfo Info);
   bool diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
                                     DeclarationName Name,

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jun 18 15:15:12 2013
@@ -4354,21 +4354,22 @@ TryToFixInvalidVariablyModifiedTypeSourc
 }
 
 /// \brief Register the given locally-scoped extern "C" declaration so
-/// that it can be found later for redeclarations
+/// that it can be found later for redeclarations. We include any extern "C"
+/// declaration that is not visible in the translation unit here, not just
+/// function-scope declarations.
 void
-Sema::RegisterLocallyScopedExternCDecl(NamedDecl *ND,
-                                       const LookupResult &Previous,
-                                       Scope *S) {
-  assert(ND->getLexicalDeclContext()->isFunctionOrMethod() &&
-         "Decl is not a locally-scoped decl!");
+Sema::RegisterLocallyScopedExternCDecl(NamedDecl *ND, Scope *S) {
+  assert(
+      !ND->getLexicalDeclContext()->getRedeclContext()->isTranslationUnit() &&
+      "Decl is not a locally-scoped decl!");
   // Note that we have a locally-scoped external with this name.
   LocallyScopedExternCDecls[ND->getDeclName()] = ND;
 }
 
-llvm::DenseMap<DeclarationName, NamedDecl *>::iterator
-Sema::findLocallyScopedExternCDecl(DeclarationName Name) {
+NamedDecl *Sema::findLocallyScopedExternCDecl(DeclarationName Name) {
   if (ExternalSource) {
     // Load locally-scoped external decls from the external source.
+    // FIXME: This is inefficient. Maybe add a DeclContext for extern "C" decls?
     SmallVector<NamedDecl *, 4> Decls;
     ExternalSource->ReadLocallyScopedExternCDecls(Decls);
     for (unsigned I = 0, N = Decls.size(); I != N; ++I) {
@@ -4378,8 +4379,9 @@ Sema::findLocallyScopedExternCDecl(Decla
         LocallyScopedExternCDecls[Decls[I]->getDeclName()] = Decls[I];
     }
   }
-  
-  return LocallyScopedExternCDecls.find(Name);
+
+  NamedDecl *D = LocallyScopedExternCDecls.lookup(Name);
+  return D ? cast<NamedDecl>(D->getMostRecentDecl()) : 0;
 }
 
 /// \brief Diagnose function specifiers on a declaration of an identifier that
@@ -5048,11 +5050,17 @@ Sema::ActOnVariableDeclarator(Scope *S,
   ProcessPragmaWeak(S, NewVD);
   checkAttributesAfterMerging(*this, *NewVD);
 
-  // If this is a locally-scoped extern C variable, update the map of
-  // such variables.
-  if (CurContext->isFunctionOrMethod() && NewVD->isExternC() &&
-      !NewVD->isInvalidDecl())
-    RegisterLocallyScopedExternCDecl(NewVD, Previous, S);
+  // If this is the first declaration of an extern C variable that is not
+  // declared directly in the translation unit, update the map of such
+  // variables.
+  if (!CurContext->getRedeclContext()->isTranslationUnit() &&
+      !NewVD->getPreviousDecl() && !NewVD->isInvalidDecl() &&
+      // FIXME: We only check isExternC if we're in an extern C context,
+      // to avoid computing and caching an 'externally visible' flag which
+      // could change if the variable's type is not visible.
+      (!getLangOpts().CPlusPlus || NewVD->isInExternCContext()) &&
+      NewVD->isExternC())
+    RegisterLocallyScopedExternCDecl(NewVD, S);
 
   return NewVD;
 }
@@ -5360,10 +5368,9 @@ bool Sema::CheckVariableDeclaration(VarD
   // not in scope.
   bool PreviousWasHidden = false;
   if (Previous.empty() && mayConflictWithNonVisibleExternC(NewVD)) {
-    llvm::DenseMap<DeclarationName, NamedDecl *>::iterator Pos
-      = findLocallyScopedExternCDecl(NewVD->getDeclName());
-    if (Pos != LocallyScopedExternCDecls.end()) {
-      Previous.addDecl(Pos->second);
+    if (NamedDecl *ExternCPrev =
+            findLocallyScopedExternCDecl(NewVD->getDeclName())) {
+      Previous.addDecl(ExternCPrev);
       PreviousWasHidden = true;
     }
   }
@@ -6598,11 +6605,13 @@ Sema::ActOnFunctionDeclarator(Scope *S,
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
-  // If this is a locally-scoped extern C function, update the
-  // map of such names.
-  if (CurContext->isFunctionOrMethod() && NewFD->isExternC()
-      && !NewFD->isInvalidDecl())
-    RegisterLocallyScopedExternCDecl(NewFD, Previous, S);
+  // If this is the first declaration of an extern C variable that is not
+  // declared directly in the translation unit, update the map of such
+  // variables.
+  if (!CurContext->getRedeclContext()->isTranslationUnit() &&
+      !NewFD->getPreviousDecl() && NewFD->isExternC() &&
+      !NewFD->isInvalidDecl())
+    RegisterLocallyScopedExternCDecl(NewFD, S);
 
   // Set this FunctionDecl's range up to the right paren.
   NewFD->setRangeEnd(D.getSourceRange().getEnd());
@@ -6709,10 +6718,9 @@ bool Sema::CheckFunctionDeclaration(Scop
   if (Previous.empty() && mayConflictWithNonVisibleExternC(NewFD)) {
     // Since we did not find anything by this name, look for a non-visible
     // extern "C" declaration with the same name.
-    llvm::DenseMap<DeclarationName, NamedDecl *>::iterator Pos
-      = findLocallyScopedExternCDecl(NewFD->getDeclName());
-    if (Pos != LocallyScopedExternCDecls.end())
-      Previous.addDecl(Pos->second);
+    if (NamedDecl *ExternCPrev =
+            findLocallyScopedExternCDecl(NewFD->getDeclName()))
+      Previous.addDecl(ExternCPrev);
   }
 
   // Filter out any non-conflicting previous declarations.
@@ -9063,12 +9071,10 @@ NamedDecl *Sema::ImplicitlyDefineFunctio
   // function, see whether there was a locally-scoped declaration of
   // this name as a function or variable. If so, use that
   // (non-visible) declaration, and complain about it.
-  llvm::DenseMap<DeclarationName, NamedDecl *>::iterator Pos
-    = findLocallyScopedExternCDecl(&II);
-  if (Pos != LocallyScopedExternCDecls.end()) {
-    Diag(Loc, diag::warn_use_out_of_scope_declaration) << Pos->second;
-    Diag(Pos->second->getLocation(), diag::note_previous_declaration);
-    return Pos->second;
+  if (NamedDecl *ExternCPrev = findLocallyScopedExternCDecl(&II)) {
+    Diag(Loc, diag::warn_use_out_of_scope_declaration) << ExternCPrev;
+    Diag(ExternCPrev->getLocation(), diag::note_previous_declaration);
+    return ExternCPrev;
   }
 
   // Extension in C99.  Legal in C90, but warn about it.

Modified: cfe/trunk/test/CXX/drs/dr0xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr0xx.cpp?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr0xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr0xx.cpp Tue Jun 18 15:15:12 2013
@@ -3,19 +3,19 @@
 // RUN: %clang_cc1 -std=c++1y %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 
 namespace dr1 { // dr1: no
-  namespace X { extern "C" void dr1_f(int a = 1); } // expected-note 2{{candidate}} expected-note {{conflicting}}
-  namespace Y { extern "C" void dr1_f(int a = 2); } // expected-note 2{{candidate}} expected-note {{target}}
+  namespace X { extern "C" void dr1_f(int a = 1); }
+  namespace Y { extern "C" void dr1_f(int a = 2); }
   using X::dr1_f; using Y::dr1_f;
   void g() {
-    // FIXME: The first of these two should be accepted.
-    dr1_f(0); // expected-error {{ambiguous}}
-    dr1_f(); // expected-error {{ambiguous}}
+    dr1_f(0);
+    // FIXME: This should be rejected, due to the ambiguous default argument.
+    dr1_f();
   }
   namespace X {
-    using Y::dr1_f; // expected-error {{conflicts with declaration already in scope}}
+    using Y::dr1_f;
     void h() {
-      // FIXME: The second of these two should be rejected.
       dr1_f(0);
+      // FIXME: This should be rejected, due to the ambiguous default argument.
       dr1_f();
     }
   }
@@ -129,22 +129,19 @@ namespace dr12 { // dr12: sup 239
   }
 }
 
-namespace dr14 { // dr14: no
-  namespace X { extern "C" int dr14_f(); } // expected-note {{candidate}}
-  namespace Y { extern "C" int dr14_f(); } // expected-note {{candidate}}
+namespace dr14 { // dr14: yes
+  namespace X { extern "C" int dr14_f(); }
+  namespace Y { extern "C" int dr14_f(); }
   using namespace X;
   using namespace Y;
-  // FIXME: This should be accepted, name lookup only finds one function (in two
-  // different namespaces).
-  int k = dr14_f(); // expected-error {{ambiguous}}
+  int k = dr14_f();
 
   class C {
-    int k; // expected-note {{here}}
+    int k;
     friend int Y::dr14_f();
   } c;
   namespace Z {
-    // FIXME: This should be accepted, this function is a friend.
-    extern "C" int dr14_f() { return c.k; } // expected-error {{private}}
+    extern "C" int dr14_f() { return c.k; }
   }
 
   namespace X { typedef int T; typedef int U; } // expected-note {{candidate}}

Modified: cfe/trunk/test/CXX/drs/dr1xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr1xx.cpp?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr1xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr1xx.cpp Tue Jun 18 15:15:12 2013
@@ -9,15 +9,14 @@ namespace dr100 { // dr100: yes
   B<"bar"> b; // expected-error {{does not refer to any declaration}}
 }
 
-namespace dr101 { // dr101: no
-  // FIXME: This is valid.
-  extern "C" void dr101_f(); // expected-note {{conflicting declaration}}
+namespace dr101 { // dr101: yes
+  extern "C" void dr101_f();
   typedef unsigned size_t;
   namespace X {
-    extern "C" void dr101_f(); // expected-note {{target of using declaration}}
+    extern "C" void dr101_f();
     typedef unsigned size_t;
   }
-  using X::dr101_f; // expected-error {{conflicts with declaration already in scope}}
+  using X::dr101_f;
   using X::size_t;
 }
 

Modified: cfe/trunk/test/SemaCXX/PR10447.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR10447.cpp?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/PR10447.cpp (original)
+++ cfe/trunk/test/SemaCXX/PR10447.cpp Tue Jun 18 15:15:12 2013
@@ -4,7 +4,7 @@
 // PR12223
 namespace test1 {
   namespace N {
-    extern "C" void f(struct S*);
+    extern "C" void f_test1(struct S*);
     void g(S*);
   }
   namespace N {
@@ -17,7 +17,7 @@ namespace test1 {
 // PR10447
 namespace test2 {
   extern "C" {
-    void f(struct Bar*) { }
+    void f_test2(struct Bar*) { }
     test2::Bar *ptr;
   }
 }

Modified: cfe/trunk/test/SemaCXX/extern-c.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/extern-c.cpp?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/extern-c.cpp (original)
+++ cfe/trunk/test/SemaCXX/extern-c.cpp Tue Jun 18 15:15:12 2013
@@ -2,7 +2,7 @@
 
 namespace test1 {
   extern "C" {
-    void f() {
+    void test1_f() {
       void test1_g(int); // expected-note {{previous declaration is here}}
     }
   }
@@ -11,7 +11,7 @@ int test1_g(int); // expected-error {{fu
 
 namespace test2 {
   extern "C" {
-    void f() {
+    void test2_f() {
       extern int test2_x; // expected-note {{previous definition is here}}
     }
   }
@@ -20,7 +20,7 @@ float test2_x; // expected-error {{redef
 
 namespace test3 {
   extern "C" {
-    void f() {
+    void test3_f() {
       extern int test3_b; // expected-note {{previous definition is here}}
     }
   }
@@ -56,3 +56,53 @@ namespace foo {
     extern float test6_b;
   }
 }
+
+namespace linkage {
+  namespace redecl {
+    extern "C" {
+      static void linkage_redecl();
+      static void linkage_redecl(int);
+      void linkage_redecl(); // ok, still not extern "C"
+      void linkage_redecl(int); // ok, still not extern "C"
+      void linkage_redecl(float); // expected-note {{previous}}
+      void linkage_redecl(double); // expected-error {{conflicting types}}
+    }
+  }
+  namespace from_outer {
+    void linkage_from_outer_1();
+    void linkage_from_outer_2(); // expected-note {{previous}}
+    extern "C" {
+      void linkage_from_outer_1(int); // expected-note {{previous}}
+      void linkage_from_outer_1(); // expected-error {{conflicting types}}
+      void linkage_from_outer_2(); // expected-error {{different language linkage}}
+    }
+  }
+  namespace mixed {
+    extern "C" {
+      void linkage_mixed_1();
+      static void linkage_mixed_1(int);
+
+      static void linkage_mixed_2(int);
+      void linkage_mixed_2();
+    }
+  }
+  namespace across_scopes {
+    namespace X {
+      extern "C" void linkage_across_scopes_f() {
+        void linkage_across_scopes_g(); // expected-note {{previous}}
+      }
+    }
+    namespace Y {
+      extern "C" void linkage_across_scopes_g(int); // expected-error {{conflicting}}
+    }
+  }
+}
+
+void lookup_in_global_f();
+namespace lookup_in_global {
+  void lookup_in_global_f();
+  extern "C" {
+    // FIXME: We should reject this.
+    void lookup_in_global_f(int);
+  }
+}

Modified: cfe/trunk/www/cxx_dr_status.html
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_dr_status.html?rev=184223&r1=184222&r2=184223&view=diff
==============================================================================
--- cfe/trunk/www/cxx_dr_status.html (original)
+++ cfe/trunk/www/cxx_dr_status.html Tue Jun 18 15:15:12 2013
@@ -122,7 +122,7 @@
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#14">14</a></td>
     <td>NAD</td>
     <td>extern "C" functions and declarations in different namespaces</td>
-    <td class="none" align="center">No</td>
+    <td class="full" align="center">Yes</td>
   </tr>
   <tr>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#15">15</a></td>
@@ -644,7 +644,7 @@
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#101">101</a></td>
     <td>TC1</td>
     <td>Redeclaration of extern "C" names via using-declarations</td>
-    <td class="none" align="center">No</td>
+    <td class="full" align="center">Yes</td>
   </tr>
   <tr>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#102">102</a></td>





More information about the cfe-commits mailing list