From b65e8fe143f8822ada578063f8d4b65db61b2788 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 1 Apr 2013 18:34:28 +0000 Subject: [PATCH] Only merge down a variable type if the previous declaration was visible. There's a lot of potential badness in how we're modelling these things, but getting this much correct is reasonably easy. rdar://13535367 llvm-svn: 178488 --- clang/include/clang/Sema/Sema.h | 5 +-- clang/lib/Sema/SemaDecl.cpp | 49 +++++++++++++++++++++----- clang/test/CXX/basic/basic.link/p6.cpp | 43 ++++++++++++++++++++++ clang/test/Sema/extern-redecl.c | 11 ++++++ 4 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 clang/test/CXX/basic/basic.link/p6.cpp diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 049290b5bb1b..a8f18fd571cc 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1759,8 +1759,9 @@ public: bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old, Scope *S); void mergeObjCMethodDecls(ObjCMethodDecl *New, ObjCMethodDecl *Old); - void MergeVarDecl(VarDecl *New, LookupResult &OldDecls); - void MergeVarDeclTypes(VarDecl *New, VarDecl *Old); + void MergeVarDecl(VarDecl *New, LookupResult &OldDecls, + bool OldDeclsWereHidden); + void MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldIsHidden); void MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old); bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, Scope *S); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 7c109a478f27..795d97d3eaf5 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2773,7 +2773,7 @@ void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod, /// Declarations using the auto type specifier (C++ [decl.spec.auto]) call back /// to here in AddInitializerToDecl. We can't check them before the initializer /// is attached. -void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old) { +void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldWasHidden) { if (New->isInvalidDecl() || Old->isInvalidDecl()) return; @@ -2820,7 +2820,11 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old) { Diag(Old->getLocation(), diag::note_previous_definition); return New->setInvalidDecl(); } - New->setType(MergedT); + + // Don't actually update the type on the new declaration if the old + // declaration was a extern declaration in a different scope. + if (!OldWasHidden) + New->setType(MergedT); } /// MergeVarDecl - We just parsed a variable 'New' which has the same name @@ -2831,7 +2835,8 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old) { /// FinalizeDeclaratorGroup. Unfortunately, we can't analyze tentative /// definitions here, since the initializer hasn't been attached. /// -void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { +void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous, + bool PreviousWasHidden) { // If the new decl is already invalid, don't do any other checking. if (New->isInvalidDecl()) return; @@ -2871,7 +2876,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { } // Merge the types. - MergeVarDeclTypes(New, Old); + MergeVarDeclTypes(New, Old, PreviousWasHidden); if (New->isInvalidDecl()) return; @@ -5206,13 +5211,39 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD, NewVD->setTypeSourceInfo(FixedTInfo); } + // If we did not find anything by this name, look for a non-visible + // extern "C" declaration with the same name. + // + // Clang has a lot of problems with extern local declarations. + // The actual standards text here is: + // + // C++11 [basic.link]p6: + // The name of a function declared in block scope and the name + // of a variable declared by a block scope extern declaration + // have linkage. If there is a visible declaration of an entity + // with linkage having the same name and type, ignoring entities + // declared outside the innermost enclosing namespace scope, the + // block scope declaration declares that same entity and + // receives the linkage of the previous declaration. + // + // C11 6.2.7p4: + // For an identifier with internal or external linkage declared + // in a scope in which a prior declaration of that identifier is + // visible, if the prior declaration specifies internal or + // external linkage, the type of the identifier at the later + // declaration becomes the composite type. + // + // The most important point here is that we're not allowed to + // update our understanding of the type according to declarations + // not in scope. + bool PreviousWasHidden = false; if (Previous.empty() && mayConflictWithNonVisibleExternC(NewVD)) { - // Since we did not find anything by this name, look for a non-visible - // extern "C" declaration with the same name. llvm::DenseMap::iterator Pos = findLocallyScopedExternCDecl(NewVD->getDeclName()); - if (Pos != LocallyScopedExternCDecls.end()) + if (Pos != LocallyScopedExternCDecls.end()) { Previous.addDecl(Pos->second); + PreviousWasHidden = true; + } } // Filter out any non-conflicting previous declarations. @@ -5245,7 +5276,7 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD, } if (!Previous.empty()) { - MergeVarDecl(NewVD, Previous); + MergeVarDecl(NewVD, Previous, PreviousWasHidden); return true; } return false; @@ -7277,7 +7308,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, // If this is a redeclaration, check that the type we just deduced matches // the previously declared type. if (VarDecl *Old = VDecl->getPreviousDecl()) - MergeVarDeclTypes(VDecl, Old); + MergeVarDeclTypes(VDecl, Old, /*OldWasHidden*/ false); } if (VDecl->isLocalVarDecl() && VDecl->hasExternalStorage()) { diff --git a/clang/test/CXX/basic/basic.link/p6.cpp b/clang/test/CXX/basic/basic.link/p6.cpp new file mode 100644 index 000000000000..8faec76fb3f1 --- /dev/null +++ b/clang/test/CXX/basic/basic.link/p6.cpp @@ -0,0 +1,43 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// C++11 [basic.link]p6: +// The name of a function declared in block scope and the name +// of a variable declared by a block scope extern declaration +// have linkage. If there is a visible declaration of an entity +// with linkage having the same name and type, ignoring entities +// declared outside the innermost enclosing namespace scope, the +// block scope declaration declares that same entity and +// receives the linkage of the previous declaration. + +// rdar://13535367 +namespace test0 { + extern "C" int test0_array[]; + void declare() { extern int test0_array[100]; } + extern "C" int test0_array[]; + int value = sizeof(test0_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}} +} + +namespace test1 { + extern "C" int test1_array[]; + void test() { + { extern int test1_array[100]; } + extern int test1_array[]; + int x = sizeof(test1_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}} + } +} + +namespace test2 { + void declare() { extern int test2_array[100]; } + extern int test2_array[]; + int value = sizeof(test2_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}} +} + +namespace test3 { + void test() { + { extern int test3_array[100]; } + extern int test3_array[]; + int x = sizeof(test3_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}} + } +} + + diff --git a/clang/test/Sema/extern-redecl.c b/clang/test/Sema/extern-redecl.c index ae4386eaae71..9a085de0c001 100644 --- a/clang/test/Sema/extern-redecl.c +++ b/clang/test/Sema/extern-redecl.c @@ -22,3 +22,14 @@ int PR10013(void) { static int test1_a[]; // expected-warning {{tentative array definition assumed to have one element}} extern int test1_a[]; + +// rdar://13535367 +void test2declarer() { extern int test2_array[100]; } +extern int test2_array[]; +int test2v = sizeof(test2_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}} + +void test3declarer() { + { extern int test3_array[100]; } + extern int test3_array[]; + int x = sizeof(test3_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}} +}