> On Dec 9, 2015, at 2:49 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > > On Wed, Dec 9, 2015 at 2:00 PM, Ben Langmuir <blangm...@apple.com > <mailto:blangm...@apple.com>> wrote: > >> On Dec 9, 2015, at 1:36 PM, Richard Smith <rich...@metafoo.co.uk >> <mailto:rich...@metafoo.co.uk>> wrote: >> >> On Wed, Dec 9, 2015 at 11:55 AM, Ben Langmuir via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> > On Dec 9, 2015, at 11:07 AM, Ben Langmuir <blangm...@apple.com >> > <mailto:blangm...@apple.com>> wrote: >> > >> > Hey Richard, >> > >> > This caused a new error for the following code: >> > >> > @import Foo.X; // declaration of ‘struct foo’ from Foo.Y is not visible >> > yet, but the pcm is loaded. >> > struct foo *bar; // declares ‘struct foo’ >> > @import Foo.Y; // also declares ‘struct foo’ >> > >> > void useFoo(struct foo *x); // error: reference to ‘foo’ is ambiguous >> > >> > This seems to be specific to declaring the tag with an elaborated type >> > specifier that is not just ‘struct foo;’. Any idea what went wrong? I’m >> > trying to track this down and fix it. >> >> It’s also specific to non-C++ language modes. In C++ we seem to cheat and >> make a second lookup that has ForRedeclaration set (SemaDecl.cpp:12122) >> which then turns this into a use of the hidden declaration rather than >> creating a new declaration or redeclaration. I call this “cheating” because >> the comments imply this second lookup is for diagnostic purposes, but it >> clearly has a semantic affect in this case. >> >> Well, this comes back to our handling of C structs and modules being a >> little incoherent. Suppose Foo.Y defines one 'struct foo', and we define a >> completely different 'struct foo': >> >> // Foo.Y >> struct foo { int n; }; >> >> // my.c >> @import Foo.X; >> struct foo *bar; >> struct foo { double d; }; >> // Don't import Foo.Y >> >> If this is not an error, then our declaration of 'struct foo' cannot be the >> same type as Foo.Y. The usual C rule for structs is that you can have >> multiple conflicting definitions in different TUs. Such declarations declare >> different types, but they're compatible types (and thus can be used >> interchangeably) if they're structurally equivalent. >> >> I think implementing that rule for C is the right way to go here. Either >> that, or we extend C++'s ODR semantics to C modules, but given that the C >> standard gives us a completely reasonable rule here, it seems more >> appropriate to follow it. > > + Doug > > To make sure I understand: if we followed the C rules then in my original > example we would get a fresh declaration here: > struct foo *x; > > and then when we import > @import Foo.Y; > > we would try to merge the types, and if we had more than one definition we > would diagnose if they aren’t structurally equivalent. > > No. At the point of import, the name 'foo' would name two different structs. > If at some later point we look up 'foo' in the tag name space, name lookup > would fold together the lookup results if they refer to compatible types (and > if they're not compatible, you'd get an ambiguity error at the point of use). > Likewise, ASTContext::typesAreCompatible would be extended to treat such > types as compatible in the cases where the C standard says they are > compatible (presumably with a cache so we don't repeatedly check for the same > pair of types).
Ah, thanks for explaining. > > Whereas if we used the ODR we could use the hidden declaration directly like > we do in C++, > > That would cause my example above to silently do the wrong thing. You'd get a > struct foo with a single member 'n', despite no such member being visible. In > C++, the code would already be ill-formed (NDR) due to violating the ODR, so > that's less terrible (but still pretty bad). In C, where the corresponding > code not using modules is valid, such behavior is pretty hard to defend. > > although perhaps looking it up with a for-redeclaration lookup wouldn’t be > sufficient if we fixed the linkage of the structs as you mention below. > > Switching to structural equivalence checking seems like a big change to make > just to fix this regression. Even if that’s what we want I’m not sure I > could sign up for it right now. Is there a more tactical way we could fix > this in the interim? > > Adding the for-redeclaration lookup to C seems fine to me as a short-term > workaround. (This will break again if/when we fix the linkage of C types, but > I don't think we have a reason to do that until we have the type > compatibility rules implemented.) Okay, I did this in r255267. >> I guess the reason this started failing after r252960 is because of this >> change: >> > This also removes the prior special case for tag lookup, which made some >> > cases >> > of this work, but also led to bizarre, bogus "must use 'struct' to refer >> > to type >> > 'Foo' in this scope" diagnostics in C++. >> >> We could do the for-redeclaration lookup outside of C++ too, which will fix >> this case, >> >> I don't think so: structs in C don't have linkage, so the for-redeclaration >> lookup won't find unimported structs (though it's possible that we get the >> linkage calculation for C structs wrong). > > It does find the struct and the hidden decl is considered “externally > visible” so I guess that’s incorrect then? Indeed doing this extra lookup > does fix my test case (and doesn’t break any other regression tests we have). > > Yes, that seems like a bug. (It would presumably break code such as my > example above...) > > >> but I’m not sure if this is the correct fix. >> >> Thoughts? >> >> Ben >> >> > >> > Ben >> > >> >> On Nov 12, 2015, at 2:19 PM, Richard Smith via cfe-commits >> >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> >> >> >> Author: rsmith >> >> Date: Thu Nov 12 16:19:45 2015 >> >> New Revision: 252960 >> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=252960&view=rev >> >> <http://llvm.org/viewvc/llvm-project?rev=252960&view=rev> >> >> Log: >> >> [modules] Simplify and generalize the existing rule for finding hidden >> >> declarations in redeclaration lookup. A declaration is now visible to >> >> lookup if: >> >> >> >> * It is visible (not in a module, or in an imported module), or >> >> * We're doing redeclaration lookup and it's externally-visible, or >> >> * We're doing typo correction and looking for unimported decls. >> >> >> >> We now support multiple modules having different internal-linkage or >> >> no-linkage >> >> definitions of the same name for all entities, not just for functions, >> >> variables, and some typedefs. As previously, if multiple such entities are >> >> visible, any attempt to use them will result in an ambiguity error. >> >> >> >> This patch fixes the linkage calculation for a number of entities where we >> >> previously didn't need to get it right (using-declarations, namespace >> >> aliases, >> >> and so on). It also classifies enumerators as always having no linkage, >> >> which >> >> is a slight deviation from the C++ standard's definition, but not an >> >> observable >> >> change outside modules (this change is being discussed on the -core >> >> reflector >> >> currently). >> >> >> >> This also removes the prior special case for tag lookup, which made some >> >> cases >> >> of this work, but also led to bizarre, bogus "must use 'struct' to refer >> >> to type >> >> 'Foo' in this scope" diagnostics in C++. >> >> >> >> Added: >> >> cfe/trunk/test/Modules/Inputs/no-linkage/ >> >> cfe/trunk/test/Modules/Inputs/no-linkage/decls.h >> >> cfe/trunk/test/Modules/Inputs/no-linkage/empty.h >> >> cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap >> >> cfe/trunk/test/Modules/no-linkage.cpp >> >> Modified: >> >> cfe/trunk/include/clang/Sema/Lookup.h >> >> cfe/trunk/lib/AST/Decl.cpp >> >> cfe/trunk/lib/Sema/SemaDecl.cpp >> >> cfe/trunk/lib/Sema/SemaDeclCXX.cpp >> >> cfe/trunk/test/Index/linkage.c >> >> cfe/trunk/test/Index/usrs.m >> >> cfe/trunk/test/Modules/decldef.m >> >> cfe/trunk/test/Modules/merge-enumerators.cpp >> >> cfe/trunk/test/Modules/module-private.cpp >> >> cfe/trunk/test/Modules/submodule-visibility-cycles.cpp >> >> cfe/trunk/test/Modules/submodules-merge-defs.cpp >> >> >> >> Modified: cfe/trunk/include/clang/Sema/Lookup.h >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/include/clang/Sema/Lookup.h (original) >> >> +++ cfe/trunk/include/clang/Sema/Lookup.h Thu Nov 12 16:19:45 2015 >> >> @@ -139,8 +139,7 @@ public: >> >> Redecl(Redecl != Sema::NotForRedeclaration), >> >> HideTags(true), >> >> Diagnose(Redecl == Sema::NotForRedeclaration), >> >> - AllowHidden(Redecl == Sema::ForRedeclaration), >> >> - AllowHiddenInternal(AllowHidden), >> >> + AllowHidden(false), >> >> Shadowed(false) >> >> { >> >> configure(); >> >> @@ -162,8 +161,7 @@ public: >> >> Redecl(Redecl != Sema::NotForRedeclaration), >> >> HideTags(true), >> >> Diagnose(Redecl == Sema::NotForRedeclaration), >> >> - AllowHidden(Redecl == Sema::ForRedeclaration), >> >> - AllowHiddenInternal(AllowHidden), >> >> + AllowHidden(false), >> >> Shadowed(false) >> >> { >> >> configure(); >> >> @@ -184,7 +182,6 @@ public: >> >> HideTags(Other.HideTags), >> >> Diagnose(false), >> >> AllowHidden(Other.AllowHidden), >> >> - AllowHiddenInternal(Other.AllowHiddenInternal), >> >> Shadowed(false) >> >> {} >> >> >> >> @@ -226,27 +223,16 @@ public: >> >> /// \brief Specify whether hidden declarations are visible, e.g., >> >> /// for recovery reasons. >> >> void setAllowHidden(bool AH) { >> >> - AllowHiddenInternal = AllowHidden = AH; >> >> - } >> >> - >> >> - /// \brief Specify whether hidden internal declarations are visible. >> >> - void setAllowHiddenInternal(bool AHI) { >> >> - AllowHiddenInternal = AHI; >> >> + AllowHidden = AH; >> >> } >> >> >> >> /// \brief Determine whether this lookup is permitted to see hidden >> >> /// declarations, such as those in modules that have not yet been >> >> imported. >> >> bool isHiddenDeclarationVisible(NamedDecl *ND) const { >> >> - // If a using-shadow declaration is hidden, it's never visible, not >> >> - // even to redeclaration lookup. >> >> - // FIXME: Should this apply to typedefs and namespace aliases too? >> >> - if (isa<UsingShadowDecl>(ND) && LookupKind != >> >> Sema::LookupUsingDeclName) >> >> - return false; >> >> - return (AllowHidden && >> >> - (AllowHiddenInternal || ND->isExternallyVisible())) || >> >> - LookupKind == Sema::LookupTagName; >> >> + return AllowHidden || >> >> + (isForRedeclaration() && ND->isExternallyVisible()); >> >> } >> >> - >> >> + >> >> /// Sets whether tag declarations should be hidden by non-tag >> >> /// declarations during resolution. The default is true. >> >> void setHideTags(bool Hide) { >> >> @@ -317,7 +303,8 @@ public: >> >> if (!D->isInIdentifierNamespace(IDNS)) >> >> return nullptr; >> >> >> >> - if (isHiddenDeclarationVisible(D) || isVisible(getSema(), D)) >> >> + if (!D->isHidden() || isHiddenDeclarationVisible(D) || >> >> + isVisibleSlow(getSema(), D)) >> >> return D; >> >> >> >> return getAcceptableDeclSlow(D); >> >> @@ -526,7 +513,6 @@ public: >> >> /// \brief Change this lookup's redeclaration kind. >> >> void setRedeclarationKind(Sema::RedeclarationKind RK) { >> >> Redecl = RK; >> >> - AllowHiddenInternal = AllowHidden = (RK == Sema::ForRedeclaration); >> >> configure(); >> >> } >> >> >> >> @@ -698,9 +684,6 @@ private: >> >> /// \brief True if we should allow hidden declarations to be 'visible'. >> >> bool AllowHidden; >> >> >> >> - /// \brief True if we should allow hidden internal declarations to be >> >> visible. >> >> - bool AllowHiddenInternal; >> >> - >> >> /// \brief True if the found declarations were shadowed by some other >> >> /// declaration that we skipped. This only happens when \c LookupKind >> >> /// is \c LookupRedeclarationWithLinkage. >> >> >> >> Modified: cfe/trunk/lib/AST/Decl.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/lib/AST/Decl.cpp (original) >> >> +++ cfe/trunk/lib/AST/Decl.cpp Thu Nov 12 16:19:45 2015 >> >> @@ -635,6 +635,8 @@ static LinkageInfo getLVForNamespaceScop >> >> if (D->isInAnonymousNamespace()) { >> >> const auto *Var = dyn_cast<VarDecl>(D); >> >> const auto *Func = dyn_cast<FunctionDecl>(D); >> >> + // FIXME: In C++11 onwards, anonymous namespaces should give decls >> >> + // within them internal linkage, not unique external linkage. >> >> if ((!Var || !isFirstInExternCContext(Var)) && >> >> (!Func || !isFirstInExternCContext(Func))) >> >> return LinkageInfo::uniqueExternal(); >> >> @@ -821,10 +823,14 @@ static LinkageInfo getLVForNamespaceScop >> >> } else if (isa<ObjCInterfaceDecl>(D)) { >> >> // fallout >> >> >> >> + } else if (auto *TD = dyn_cast<TypedefNameDecl>(D)) { >> >> + // A typedef declaration has linkage if it gives a type a name for >> >> + // linkage purposes. >> >> + if (!TD->getAnonDeclWithTypedefName(/*AnyRedecl*/true)) >> >> + return LinkageInfo::none(); >> >> + >> >> // Everything not covered here has no linkage. >> >> } else { >> >> - // FIXME: A typedef declaration has linkage if it gives a type a >> >> name for >> >> - // linkage purposes. >> >> return LinkageInfo::none(); >> >> } >> >> >> >> @@ -1226,8 +1232,32 @@ static LinkageInfo computeLVForDecl(cons >> >> switch (D->getKind()) { >> >> default: >> >> break; >> >> + >> >> + // Per C++ [basic.link]p2, only the names of objects, references, >> >> + // functions, types, templates, namespaces, and values ever have >> >> linkage. >> >> + // >> >> + // Note that the name of a typedef, namespace alias, using >> >> declaration, >> >> + // and so on are not the name of the corresponding type, namespace, >> >> or >> >> + // declaration, so they do *not* have linkage. >> >> + case Decl::EnumConstant: // FIXME: This has linkage, but that's dumb. >> >> + case Decl::ImplicitParam: >> >> + case Decl::Label: >> >> + case Decl::NamespaceAlias: >> >> case Decl::ParmVar: >> >> + case Decl::Using: >> >> + case Decl::UsingShadow: >> >> + case Decl::UsingDirective: >> >> return LinkageInfo::none(); >> >> + >> >> + case Decl::Typedef: >> >> + case Decl::TypeAlias: >> >> + // A typedef declaration has linkage if it gives a type a name for >> >> + // linkage purposes. >> >> + if (!cast<TypedefNameDecl>(D) >> >> + ->getAnonDeclWithTypedefName(/*AnyRedecl*/true)) >> >> + return LinkageInfo::none(); >> >> + break; >> >> + >> >> case Decl::TemplateTemplateParm: // count these as external >> >> case Decl::NonTypeTemplateParm: >> >> case Decl::ObjCAtDefsField: >> >> >> >> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >> >> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Nov 12 16:19:45 2015 >> >> @@ -4819,12 +4819,6 @@ NamedDecl *Sema::HandleDeclarator(Scope >> >> LookupResult Previous(*this, NameInfo, LookupOrdinaryName, >> >> ForRedeclaration); >> >> >> >> - // If we're hiding internal-linkage symbols in modules from >> >> redeclaration >> >> - // lookup, let name lookup know. >> >> - if ((getLangOpts().Modules || getLangOpts().ModulesLocalVisibility) && >> >> - D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef) >> >> - Previous.setAllowHiddenInternal(false); >> >> - >> >> // See if this is a redefinition of a variable in the same scope. >> >> if (!D.getCXXScopeSpec().isSet()) { >> >> bool IsLinkageLookup = false; >> >> >> >> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) >> >> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Nov 12 16:19:45 2015 >> >> @@ -7206,23 +7206,13 @@ Decl *Sema::ActOnStartNamespaceDef(Scope >> >> // treated as an original-namespace-name. >> >> // >> >> // Since namespace names are unique in their scope, and we don't >> >> - // look through using directives, just look for any ordinary names. >> >> - >> >> - const unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_Member | >> >> - Decl::IDNS_Type | Decl::IDNS_Using | Decl::IDNS_Tag | >> >> - Decl::IDNS_Namespace; >> >> - NamedDecl *PrevDecl = nullptr; >> >> - DeclContext::lookup_result R = >> >> CurContext->getRedeclContext()->lookup(II); >> >> - for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; >> >> - ++I) { >> >> - if ((*I)->getIdentifierNamespace() & IDNS) { >> >> - PrevDecl = *I; >> >> - break; >> >> - } >> >> - } >> >> - >> >> + // look through using directives, just look for any ordinary names >> >> + // as if by qualified name lookup. >> >> + LookupResult R(*this, II, IdentLoc, LookupOrdinaryName, >> >> ForRedeclaration); >> >> + LookupQualifiedName(R, CurContext->getRedeclContext()); >> >> + NamedDecl *PrevDecl = R.getAsSingle<NamedDecl>(); >> >> PrevNS = dyn_cast_or_null<NamespaceDecl>(PrevDecl); >> >> - >> >> + >> >> if (PrevNS) { >> >> // This is an extended namespace definition. >> >> if (IsInline != PrevNS->isInline()) >> >> >> >> Modified: cfe/trunk/test/Index/linkage.c >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/linkage.c?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/linkage.c?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Index/linkage.c (original) >> >> +++ cfe/trunk/test/Index/linkage.c Thu Nov 12 16:19:45 2015 >> >> @@ -20,7 +20,7 @@ void f16(void) { >> >> >> >> >> >> // CHECK: EnumDecl=Baz:3:6 (Definition)linkage=External >> >> -// CHECK: EnumConstantDecl=Qux:3:12 (Definition)linkage=External >> >> +// CHECK: EnumConstantDecl=Qux:3:12 (Definition)linkage=NoLinkage >> >> // CHECK: VarDecl=x:4:5linkage=External >> >> // CHECK: FunctionDecl=foo:5:6linkage=External >> >> // CHECK: VarDecl=w:6:12linkage=Internal >> >> >> >> Modified: cfe/trunk/test/Index/usrs.m >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/usrs.m?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/usrs.m?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Index/usrs.m (original) >> >> +++ cfe/trunk/test/Index/usrs.m Thu Nov 12 16:19:45 2015 >> >> @@ -119,7 +119,7 @@ int test_multi_declaration(void) { >> >> // CHECK: usrs.m c:@SA@MyStruct Extent=[15:9 - 18:2] >> >> // CHECK: usrs.m c:@SA@MyStruct@FI@wa Extent=[16:3 - 16:9] >> >> // CHECK: usrs.m c:@SA@MyStruct@FI@moo Extent=[17:3 - 17:10] >> >> -// CHECK: usrs.m c:usrs.m@T@MyStruct Extent=[15:1 - 18:11] >> >> +// CHECK: usrs.m c:@T@MyStruct Extent=[15:1 - 18:11] >> >> // CHECK: usrs.m c:@E@Pizza Extent=[20:1 - 23:2] >> >> // CHECK: usrs.m c:@E@Pizza@CHEESE Extent=[21:3 - 21:9] >> >> // CHECK: usrs.m c:@E@Pizza@MUSHROOMS Extent=[22:3 - 22:12] >> >> >> >> Added: cfe/trunk/test/Modules/Inputs/no-linkage/decls.h >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/decls.h?rev=252960&view=auto >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/decls.h?rev=252960&view=auto> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/Inputs/no-linkage/decls.h (added) >> >> +++ cfe/trunk/test/Modules/Inputs/no-linkage/decls.h Thu Nov 12 16:19:45 >> >> 2015 >> >> @@ -0,0 +1,6 @@ >> >> +namespace RealNS { int UsingDecl; } >> >> +namespace NS = RealNS; >> >> +typedef int Typedef; >> >> +using AliasDecl = int; >> >> +enum Enum { Enumerator }; >> >> +using RealNS::UsingDecl; >> >> >> >> Added: cfe/trunk/test/Modules/Inputs/no-linkage/empty.h >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/empty.h?rev=252960&view=auto >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/empty.h?rev=252960&view=auto> >> >> ============================================================================== >> >> (empty) >> >> >> >> Added: cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap?rev=252960&view=auto >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap?rev=252960&view=auto> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap (added) >> >> +++ cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap Thu Nov 12 >> >> 16:19:45 2015 >> >> @@ -0,0 +1 @@ >> >> +module M { module E { header "empty.h" } module D { header "decls.h" } } >> >> >> >> Modified: cfe/trunk/test/Modules/decldef.m >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/decldef.m?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/decldef.m?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/decldef.m (original) >> >> +++ cfe/trunk/test/Modules/decldef.m Thu Nov 12 16:19:45 2015 >> >> @@ -11,7 +11,13 @@ Def *def; >> >> #ifdef USE_EARLY >> >> A *a1; // expected-error{{declaration of 'A' must be imported from module >> >> 'decldef.Def' before it is required}} >> >> #endif >> >> -B *b1; // expected-error{{must use 'struct' tag to refer to type 'B'}} >> >> +B *b1; >> >> +#ifdef USE_EARLY >> >> +// expected-error@-2{{must use 'struct' tag to refer to type 'B'}} >> >> +#else >> >> +// expected-error@-4{{declaration of 'B' must be imported from module >> >> 'decldef.Decl' before it is required}} >> >> +// expected-note@Inputs/decl.h:2 {{previous}} >> >> +#endif >> >> @import decldef.Decl; >> >> >> >> A *a2; >> >> >> >> Modified: cfe/trunk/test/Modules/merge-enumerators.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-enumerators.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-enumerators.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/merge-enumerators.cpp (original) >> >> +++ cfe/trunk/test/Modules/merge-enumerators.cpp Thu Nov 12 16:19:45 2015 >> >> @@ -16,11 +16,13 @@ >> >> >> >> #ifdef MERGE_LATE >> >> namespace N { >> >> - // FIXME: Should we accept this and reject the usage below due to >> >> ambiguity instead? >> >> - enum { A } a; // expected-error {{redefinition of enumerator 'A'}} >> >> - // expected-note@a.h <mailto:expected-note@a.h>:1 {{here}} (from >> >> module B.b) >> >> + enum { A } a; // expected-note {{candidate}} >> >> + // expected-note@a.h <mailto:expected-note@a.h>:1 {{candidate}} (from >> >> module B.b) >> >> } >> >> #include "a.h" >> >> #endif >> >> >> >> N::E e = N::A; >> >> +#ifdef MERGE_LATE >> >> +// expected-error@-2 {{ambiguous}} >> >> +#endif >> >> >> >> Modified: cfe/trunk/test/Modules/module-private.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-private.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-private.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/module-private.cpp (original) >> >> +++ cfe/trunk/test/Modules/module-private.cpp Thu Nov 12 16:19:45 2015 >> >> @@ -12,11 +12,7 @@ void test() { >> >> } >> >> >> >> int test_broken() { >> >> - HiddenStruct hidden; // \ >> >> - // expected-error{{must use 'struct' tag to refer to type >> >> 'HiddenStruct' in this scope}} \ >> >> - // expected-error{{definition of 'HiddenStruct' must be imported}} >> >> - // expected-note@Inputs/module_private_left.h:3 {{previous definition >> >> is here}} >> >> - >> >> + HiddenStruct hidden; // expected-error{{unknown type name >> >> 'HiddenStruct'}} >> >> Integer i; // expected-error{{unknown type name 'Integer'}} >> >> >> >> int *ip = 0; >> >> >> >> Added: cfe/trunk/test/Modules/no-linkage.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/no-linkage.cpp?rev=252960&view=auto >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/no-linkage.cpp?rev=252960&view=auto> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/no-linkage.cpp (added) >> >> +++ cfe/trunk/test/Modules/no-linkage.cpp Thu Nov 12 16:19:45 2015 >> >> @@ -0,0 +1,35 @@ >> >> +// RUN: rm -rf %t >> >> +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t >> >> -fmodules-local-submodule-visibility -I%S/Inputs/no-linkage >> >> -fmodule-map-file=%S/Inputs/no-linkage/module.modulemap %s -verify >> >> + >> >> +#include "empty.h" >> >> + >> >> +namespace NS { int n; } // expected-note {{candidate}} >> >> +struct Typedef { int n; }; // expected-note {{candidate}} >> >> +int AliasDecl; // expected-note {{candidate}} >> >> +enum AlsoAnEnum { Enumerator }; // expected-note {{candidate}} >> >> +int UsingDecl; // expected-note {{candidate}} >> >> + >> >> +// expected-note@decls.h <mailto:expected-note@decls.h>:2 {{candidate}} >> >> +// expected-note@decls.h <mailto:expected-note@decls.h>:3 {{candidate}} >> >> +// expected-note@decls.h <mailto:expected-note@decls.h>:4 {{candidate}} >> >> +// expected-note@decls.h <mailto:expected-note@decls.h>:5 {{candidate}} >> >> +// expected-note@decls.h <mailto:expected-note@decls.h>:6 {{candidate}} >> >> + >> >> +void use(int); >> >> +void use_things() { >> >> + use(Typedef().n); >> >> + use(NS::n); >> >> + use(AliasDecl); >> >> + use(Enumerator); >> >> + use(UsingDecl); >> >> +} >> >> + >> >> +#include "decls.h" >> >> + >> >> +void use_things_again() { >> >> + use(Typedef().n); // expected-error {{ambiguous}} >> >> + use(NS::n); // expected-error {{ambiguous}} expected-error{{'NS' is >> >> not a class, namespace, or enumeration}} >> >> + use(AliasDecl); // expected-error {{ambiguous}} >> >> + use(Enumerator); // expected-error {{ambiguous}} >> >> + use(UsingDecl); // expected-error {{ambiguous}} >> >> +} >> >> >> >> Modified: cfe/trunk/test/Modules/submodule-visibility-cycles.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodule-visibility-cycles.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodule-visibility-cycles.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/submodule-visibility-cycles.cpp (original) >> >> +++ cfe/trunk/test/Modules/submodule-visibility-cycles.cpp Thu Nov 12 >> >> 16:19:45 2015 >> >> @@ -3,7 +3,7 @@ >> >> >> >> #include "cycle1.h" >> >> C1 c1; >> >> -C2 c2; // expected-error {{must be imported}} expected-error {{}} >> >> +C2 c2; // expected-error {{must be imported}} >> >> // expected-note@cycle2.h <mailto:expected-note@cycle2.h>:6 {{here}} >> >> >> >> #include "cycle2.h" >> >> >> >> Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=252960&r1=252959&r2=252960&view=diff >> >> >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=252960&r1=252959&r2=252960&view=diff> >> >> ============================================================================== >> >> --- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original) >> >> +++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Thu Nov 12 16:19:45 >> >> 2015 >> >> @@ -12,7 +12,7 @@ >> >> #include "indirect.h" >> >> #endif >> >> >> >> -A pre_a; // expected-error {{must use 'struct'}} >> >> +A pre_a; >> >> #ifdef IMPORT_USE_2 >> >> // expected-error-re@-2 {{must be imported from one of >> >> {{.*}}stuff.use{{.*}}stuff.use-2}} >> >> #elif EARLY_INDIRECT_INCLUDE >> >> @@ -21,29 +21,28 @@ A pre_a; // expected-error {{must use 's >> >> // expected-error@-6 {{must be imported from module 'stuff.use'}} >> >> #endif >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:1 +{{here}} >> >> +extern class A pre_a2; >> >> +int pre_use_a = use_a(pre_a2); // expected-error {{'A' must be >> >> imported}} expected-error {{'use_a' must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:2 +{{here}} >> >> -int pre_use_a = use_a(pre_a); // expected-error {{'A' must be imported}} >> >> expected-error {{'use_a' must be imported}} >> >> >> >> B::Inner2 pre_bi; // expected-error +{{must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:4 +{{here}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:17 +{{here}} >> >> -void pre_bfi(B b) { // expected-error {{must use 'class'}} >> >> expected-error +{{must be imported}} >> >> - b.f<int>(); // expected-error +{{must be imported}} expected-error >> >> +{{}} >> >> - // expected-note@defs.h <mailto:expected-note@defs.h>:19 +{{here}} >> >> +void pre_bfi(B b) { // expected-error +{{must be imported}} >> >> + b.f<int>(); // expected-error +{{}} >> >> } >> >> >> >> C_Base<1> pre_cb1; // expected-error +{{must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:23 +{{here}} >> >> -C1 pre_c1; // expected-error +{{must be imported}} expected-error {{must >> >> use 'struct'}} >> >> +C1 pre_c1; // expected-error +{{must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:25 +{{here}} >> >> -C2 pre_c2; // expected-error +{{must be imported}} expected-error {{must >> >> use 'struct'}} >> >> +C2 pre_c2; // expected-error +{{must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:26 +{{here}} >> >> >> >> D::X pre_dx; // expected-error +{{must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:28 +{{here}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:29 +{{here}} >> >> -// FIXME: We should warn that use_dx is being used without being >> >> imported. >> >> -int pre_use_dx = use_dx(pre_dx); >> >> +int pre_use_dx = use_dx(pre_dx); // ignored; pre_dx is invalid >> >> >> >> int pre_e = E(0); // expected-error {{must be imported}} >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:32 +{{here}} >> >> @@ -69,8 +68,9 @@ J<> pre_j; // expected-error {{declarati >> >> #endif >> >> // expected-note@defs.h <mailto:expected-note@defs.h>:58 +{{here}} >> >> >> >> -ScopedEnum pre_scopedenum; // expected-error {{must be imported}} >> >> expected-error {{must use 'enum'}} >> >> -// expected-note@defs.h <mailto:expected-note@defs.h>:106 {{here}} >> >> +ScopedEnum pre_scopedenum; // expected-error {{must be imported}} >> >> +// expected-note@defs.h <mailto:expected-note@defs.h>:105 0-1{{here}} >> >> +// expected-note@defs.h <mailto:expected-note@defs.h>:106 0-1{{here}} >> >> enum ScopedEnum : int; >> >> ScopedEnum pre_scopedenum_declared; // ok >> >> >> >> >> >> >> >> _______________________________________________ >> >> cfe-commits mailing list >> >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >> > >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits