Re: r315251 - [Modules TS] Module ownership semantics for redeclarations.

2017-10-10 Thread Eric Liu via cfe-commits 
No :( I tried reverting only r315256 but still got the errors.

On Tue, Oct 10, 2017 at 6:10 PM Richard Smith  wrote:

> On 10 Oct 2017 05:41, "Eric Liu via cfe-commits" <
> cfe-commits@lists.llvm.org> wrote:
>
> Hi Richard,
>
> This is breaking the boostrap stage in the internal integration. I'm
> seeing "unsupported: typedef changes linkage of anonymous type, but
> linkage was already computed" error for many `struct`s defined with
> typedef. I'm not sure if it is user code or clang that needs fixing;
> however, as there are likely many more struct definitions that would cause
> the same failure, I'll revert this commit as well as r315256, which depends
> on this. Sorry about that.
>
>
> r315256 should have fixed those errors. Did it not?
>
> I'll also send you the repros separately.
>
> Regards,
> Eric
>
> On Tue, Oct 10, 2017 at 1:42 AM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Mon Oct  9 16:42:09 2017
>> New Revision: 315251
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315251&view=rev
>> Log:
>> [Modules TS] Module ownership semantics for redeclarations.
>>
>> When declaring an entity in the "purview" of a module, it's never a
>> redeclaration of an entity in the purview of a default module or in no
>> module
>> ("in the global module"). Don't consider those other declarations as
>> possible
>> redeclaration targets if they're not visible, and reject any cases where
>> we
>> pick a prior visible declaration that violates this rule.
>>
>> Added:
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/Decl.h
>> cfe/trunk/include/clang/AST/DeclBase.h
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/include/clang/Sema/Lookup.h
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/AST/DeclBase.cpp
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> cfe/trunk/lib/Sema/SemaExprMember.cpp
>> cfe/trunk/lib/Sema/SemaLookup.cpp
>> cfe/trunk/lib/Sema/SemaOpenMP.cpp
>> cfe/trunk/lib/Sema/SemaTemplate.cpp
>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> cfe/trunk/test/SemaCXX/modules-ts.cppm
>>
>> Modified: cfe/trunk/include/clang/AST/Decl.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=315251&r1=315250&r2=315251&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>> +++ cfe/trunk/include/clang/AST/Decl.h Mon Oct  9 16:42:09 2017
>> @@ -339,6 +339,12 @@ public:
>>  return clang::isExternallyVisible(getLinkageInternal());
>>}
>>
>> +  /// Determine whether this declaration can be redeclared in a
>> +  /// different translation unit.
>> +  bool isExternallyDeclarable() const {
>> +return isExternallyVisible() && !getOwningModuleForLinkage();
>> +  }
>> +
>>/// \brief Determines the visibility of this entity.
>>Visibility getVisibility() const {
>>  return getLinkageAndVisibility().getVisibility();
>> @@ -379,10 +385,6 @@ public:
>>  return hasCachedLinkage();
>>}
>>
>> -  /// Get the module that owns this declaration for linkage purposes.
>> -  /// There only ever is such a module under the C++ Modules TS.
>> -  Module *getOwningModuleForLinkage() const;
>> -
>>/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
>>/// the underlying named decl.
>>NamedDecl *getUnderlyingDecl() {
>>
>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=315251&r1=315250&r2=315251&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclBase.h Mon Oct  9 16:42:09 2017
>> @@ -738,6 +738,10 @@ public:
>>  return isFromASTFile() ? getImportedOwningModule() :
>> getLocalOwningModule();
>>}
>>
>> +  /// Get the module that owns this declaration for linkage purposes.
>> +  /// There only ever is such a module under the C++ Modules TS.
>> +  Module *getOwningModuleForLinkage() const;
>> +
>>/// \brief Determine whether this declaration might be hidden from name
>>/// lookup. Note that the declaration might be visible even if this
>> returns
>>/// \c false, if the owning module is visible within the query context.
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clan

Re: r315251 - [Modules TS] Module ownership semantics for redeclarations.

2017-10-10 Thread Richard Smith via cfe-commits 
On 10 Oct 2017 05:41, "Eric Liu via cfe-commits" 
wrote:

Hi Richard,

This is breaking the boostrap stage in the internal integration. I'm seeing
"unsupported: typedef changes linkage of anonymous type, but linkage was
already computed" error for many `struct`s defined with typedef. I'm not
sure if it is user code or clang that needs fixing; however, as there are
likely many more struct definitions that would cause the same failure, I'll
revert this commit as well as r315256, which depends on this. Sorry about
that.


r315256 should have fixed those errors. Did it not?

I'll also send you the repros separately.

Regards,
Eric

On Tue, Oct 10, 2017 at 1:42 AM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Mon Oct  9 16:42:09 2017
> New Revision: 315251
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315251&view=rev
> Log:
> [Modules TS] Module ownership semantics for redeclarations.
>
> When declaring an entity in the "purview" of a module, it's never a
> redeclaration of an entity in the purview of a default module or in no
> module
> ("in the global module"). Don't consider those other declarations as
> possible
> redeclaration targets if they're not visible, and reject any cases where we
> pick a prior visible declaration that violates this rule.
>
> Added:
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/global-
> vs-module.cpp
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-
> vs-global.cpp
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-
> vs-module.cpp
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/include/clang/AST/DeclBase.h
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Lookup.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/AST/DeclBase.cpp
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
> cfe/trunk/lib/Sema/SemaExprMember.cpp
> cfe/trunk/lib/Sema/SemaLookup.cpp
> cfe/trunk/lib/Sema/SemaOpenMP.cpp
> cfe/trunk/lib/Sema/SemaTemplate.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/SemaCXX/modules-ts.cppm
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/Decl.h?rev=315251&r1=315250&r2=315251&view=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Mon Oct  9 16:42:09 2017
> @@ -339,6 +339,12 @@ public:
>  return clang::isExternallyVisible(getLinkageInternal());
>}
>
> +  /// Determine whether this declaration can be redeclared in a
> +  /// different translation unit.
> +  bool isExternallyDeclarable() const {
> +return isExternallyVisible() && !getOwningModuleForLinkage();
> +  }
> +
>/// \brief Determines the visibility of this entity.
>Visibility getVisibility() const {
>  return getLinkageAndVisibility().getVisibility();
> @@ -379,10 +385,6 @@ public:
>  return hasCachedLinkage();
>}
>
> -  /// Get the module that owns this declaration for linkage purposes.
> -  /// There only ever is such a module under the C++ Modules TS.
> -  Module *getOwningModuleForLinkage() const;
> -
>/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
>/// the underlying named decl.
>NamedDecl *getUnderlyingDecl() {
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/DeclBase.h?rev=315251&r1=315250&r2=315251&view=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Mon Oct  9 16:42:09 2017
> @@ -738,6 +738,10 @@ public:
>  return isFromASTFile() ? getImportedOwningModule() :
> getLocalOwningModule();
>}
>
> +  /// Get the module that owns this declaration for linkage purposes.
> +  /// There only ever is such a module under the C++ Modules TS.
> +  Module *getOwningModuleForLinkage() const;
> +
>/// \brief Determine whether this declaration might be hidden from name
>/// lookup. Note that the declaration might be visible even if this
> returns
>/// \c false, if the owning module is visible within the query context.
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticSemaKinds.td?rev=315251&r1=315250&r2=315251&view=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/Diagnost

Re: r315251 - [Modules TS] Module ownership semantics for redeclarations.

2017-10-10 Thread Hans Wennborg via cfe-commits 
Thanks for reverting.

We saw build failures in Chromium too. E.g. from
https://build.chromium.org/p/chromium.clang/builders/ToTLinux/builds/134:

In file included from
../../third_party/breakpad/breakpad/src/common/linux/symbol_upload.cc:34:
../../third_party/breakpad/breakpad/src/common/linux/symbol_upload.h:51:3:
error: unsupported: typedef changes linkage of anonymous type, but
linkage was already computed
} Options;
  ^
../../third_party/breakpad/breakpad/src/common/linux/symbol_upload.h:44:15:
note: use a tag name here to establish linkage prior to definition
typedef struct {
  ^
   Options


On Tue, Oct 10, 2017 at 5:41 AM, Eric Liu via cfe-commits
 wrote:
> Hi Richard,
>
> This is breaking the boostrap stage in the internal integration. I'm seeing
> "unsupported: typedef changes linkage of anonymous type, but linkage was
> already computed" error for many `struct`s defined with typedef. I'm not
> sure if it is user code or clang that needs fixing; however, as there are
> likely many more struct definitions that would cause the same failure, I'll
> revert this commit as well as r315256, which depends on this. Sorry about
> that.
>
> I'll also send you the repros separately.
>
> Regards,
> Eric
>
> On Tue, Oct 10, 2017 at 1:42 AM Richard Smith via cfe-commits
>  wrote:
>>
>> Author: rsmith
>> Date: Mon Oct  9 16:42:09 2017
>> New Revision: 315251
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315251&view=rev
>> Log:
>> [Modules TS] Module ownership semantics for redeclarations.
>>
>> When declaring an entity in the "purview" of a module, it's never a
>> redeclaration of an entity in the purview of a default module or in no
>> module
>> ("in the global module"). Don't consider those other declarations as
>> possible
>> redeclaration targets if they're not visible, and reject any cases where
>> we
>> pick a prior visible declaration that violates this rule.
>>
>> Added:
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/Decl.h
>> cfe/trunk/include/clang/AST/DeclBase.h
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/include/clang/Sema/Lookup.h
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/AST/DeclBase.cpp
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> cfe/trunk/lib/Sema/SemaExprMember.cpp
>> cfe/trunk/lib/Sema/SemaLookup.cpp
>> cfe/trunk/lib/Sema/SemaOpenMP.cpp
>> cfe/trunk/lib/Sema/SemaTemplate.cpp
>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> cfe/trunk/test/SemaCXX/modules-ts.cppm
>>
>> Modified: cfe/trunk/include/clang/AST/Decl.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=315251&r1=315250&r2=315251&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>> +++ cfe/trunk/include/clang/AST/Decl.h Mon Oct  9 16:42:09 2017
>> @@ -339,6 +339,12 @@ public:
>>  return clang::isExternallyVisible(getLinkageInternal());
>>}
>>
>> +  /// Determine whether this declaration can be redeclared in a
>> +  /// different translation unit.
>> +  bool isExternallyDeclarable() const {
>> +return isExternallyVisible() && !getOwningModuleForLinkage();
>> +  }
>> +
>>/// \brief Determines the visibility of this entity.
>>Visibility getVisibility() const {
>>  return getLinkageAndVisibility().getVisibility();
>> @@ -379,10 +385,6 @@ public:
>>  return hasCachedLinkage();
>>}
>>
>> -  /// Get the module that owns this declaration for linkage purposes.
>> -  /// There only ever is such a module under the C++ Modules TS.
>> -  Module *getOwningModuleForLinkage() const;
>> -
>>/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
>>/// the underlying named decl.
>>NamedDecl *getUnderlyingDecl() {
>>
>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=315251&r1=315250&r2=315251&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclBase.h Mon Oct  9 16:42:09 2017
>> @@ -738,6 +738,10 @@ public:
>>  return isFromASTFile() ? getImportedOwningModule() :
>> getLocalOwningModule();
>>}
>>
>> +  /// Get the module that owns this declaration for linkage purposes.
>> +  /// There only ever is such a module under the C++ Modules TS.
>> +  Module *getOwningModuleForLi

Re: r315251 - [Modules TS] Module ownership semantics for redeclarations.

2017-10-10 Thread NAKAMURA Takumi via cfe-commits 
FYI, I tweaked the tree in rL315283. Shall we restore it?

On Tue, Oct 10, 2017 at 9:41 PM Eric Liu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi Richard,
>
> This is breaking the boostrap stage in the internal integration. I'm
> seeing "unsupported: typedef changes linkage of anonymous type, but
> linkage was already computed" error for many `struct`s defined with
> typedef. I'm not sure if it is user code or clang that needs fixing;
> however, as there are likely many more struct definitions that would cause
> the same failure, I'll revert this commit as well as r315256, which depends
> on this. Sorry about that.
>
> I'll also send you the repros separately.
>
> Regards,
> Eric
>
> On Tue, Oct 10, 2017 at 1:42 AM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Mon Oct  9 16:42:09 2017
>> New Revision: 315251
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315251&view=rev
>> Log:
>> [Modules TS] Module ownership semantics for redeclarations.
>>
>> When declaring an entity in the "purview" of a module, it's never a
>> redeclaration of an entity in the purview of a default module or in no
>> module
>> ("in the global module"). Don't consider those other declarations as
>> possible
>> redeclaration targets if they're not visible, and reject any cases where
>> we
>> pick a prior visible declaration that violates this rule.
>>
>> Added:
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp
>>
>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/Decl.h
>> cfe/trunk/include/clang/AST/DeclBase.h
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/include/clang/Sema/Lookup.h
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/AST/DeclBase.cpp
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> cfe/trunk/lib/Sema/SemaExprMember.cpp
>> cfe/trunk/lib/Sema/SemaLookup.cpp
>> cfe/trunk/lib/Sema/SemaOpenMP.cpp
>> cfe/trunk/lib/Sema/SemaTemplate.cpp
>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> cfe/trunk/test/SemaCXX/modules-ts.cppm
>>
>> Modified: cfe/trunk/include/clang/AST/Decl.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=315251&r1=315250&r2=315251&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>> +++ cfe/trunk/include/clang/AST/Decl.h Mon Oct  9 16:42:09 2017
>> @@ -339,6 +339,12 @@ public:
>>  return clang::isExternallyVisible(getLinkageInternal());
>>}
>>
>> +  /// Determine whether this declaration can be redeclared in a
>> +  /// different translation unit.
>> +  bool isExternallyDeclarable() const {
>> +return isExternallyVisible() && !getOwningModuleForLinkage();
>> +  }
>> +
>>/// \brief Determines the visibility of this entity.
>>Visibility getVisibility() const {
>>  return getLinkageAndVisibility().getVisibility();
>> @@ -379,10 +385,6 @@ public:
>>  return hasCachedLinkage();
>>}
>>
>> -  /// Get the module that owns this declaration for linkage purposes.
>> -  /// There only ever is such a module under the C++ Modules TS.
>> -  Module *getOwningModuleForLinkage() const;
>> -
>>/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
>>/// the underlying named decl.
>>NamedDecl *getUnderlyingDecl() {
>>
>> Modified: cfe/trunk/include/clang/AST/DeclBase.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=315251&r1=315250&r2=315251&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclBase.h Mon Oct  9 16:42:09 2017
>> @@ -738,6 +738,10 @@ public:
>>  return isFromASTFile() ? getImportedOwningModule() :
>> getLocalOwningModule();
>>}
>>
>> +  /// Get the module that owns this declaration for linkage purposes.
>> +  /// There only ever is such a module under the C++ Modules TS.
>> +  Module *getOwningModuleForLinkage() const;
>> +
>>/// \brief Determine whether this declaration might be hidden from name
>>/// lookup. Note that the declaration might be visible even if this
>> returns
>>/// \c false, if the owning module is visible within the query context.
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=315251&r1=315250&r2=315251&view=diff
>>
>> =

Re: r315251 - [Modules TS] Module ownership semantics for redeclarations.

2017-10-10 Thread Eric Liu via cfe-commits 
Hi Richard,

This is breaking the boostrap stage in the internal integration. I'm seeing
"unsupported: typedef changes linkage of anonymous type, but linkage was
already computed" error for many `struct`s defined with typedef. I'm not
sure if it is user code or clang that needs fixing; however, as there are
likely many more struct definitions that would cause the same failure, I'll
revert this commit as well as r315256, which depends on this. Sorry about
that.

I'll also send you the repros separately.

Regards,
Eric

On Tue, Oct 10, 2017 at 1:42 AM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Mon Oct  9 16:42:09 2017
> New Revision: 315251
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315251&view=rev
> Log:
> [Modules TS] Module ownership semantics for redeclarations.
>
> When declaring an entity in the "purview" of a module, it's never a
> redeclaration of an entity in the purview of a default module or in no
> module
> ("in the global module"). Don't consider those other declarations as
> possible
> redeclaration targets if they're not visible, and reject any cases where we
> pick a prior visible declaration that violates this rule.
>
> Added:
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/
>
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
>
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp
>
> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/include/clang/AST/DeclBase.h
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Lookup.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/AST/DeclBase.cpp
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
> cfe/trunk/lib/Sema/SemaExprMember.cpp
> cfe/trunk/lib/Sema/SemaLookup.cpp
> cfe/trunk/lib/Sema/SemaOpenMP.cpp
> cfe/trunk/lib/Sema/SemaTemplate.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/SemaCXX/modules-ts.cppm
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=315251&r1=315250&r2=315251&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Mon Oct  9 16:42:09 2017
> @@ -339,6 +339,12 @@ public:
>  return clang::isExternallyVisible(getLinkageInternal());
>}
>
> +  /// Determine whether this declaration can be redeclared in a
> +  /// different translation unit.
> +  bool isExternallyDeclarable() const {
> +return isExternallyVisible() && !getOwningModuleForLinkage();
> +  }
> +
>/// \brief Determines the visibility of this entity.
>Visibility getVisibility() const {
>  return getLinkageAndVisibility().getVisibility();
> @@ -379,10 +385,6 @@ public:
>  return hasCachedLinkage();
>}
>
> -  /// Get the module that owns this declaration for linkage purposes.
> -  /// There only ever is such a module under the C++ Modules TS.
> -  Module *getOwningModuleForLinkage() const;
> -
>/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
>/// the underlying named decl.
>NamedDecl *getUnderlyingDecl() {
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=315251&r1=315250&r2=315251&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Mon Oct  9 16:42:09 2017
> @@ -738,6 +738,10 @@ public:
>  return isFromASTFile() ? getImportedOwningModule() :
> getLocalOwningModule();
>}
>
> +  /// Get the module that owns this declaration for linkage purposes.
> +  /// There only ever is such a module under the C++ Modules TS.
> +  Module *getOwningModuleForLinkage() const;
> +
>/// \brief Determine whether this declaration might be hidden from name
>/// lookup. Note that the declaration might be visible even if this
> returns
>/// \c false, if the owning module is visible within the query context.
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=315251&r1=315250&r2=315251&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Oct  9
> 16:42:09 2017
> @@ -4801,6 +4801,9 @@ def err_thread_non_thread : Error<
>  def err_thread_thread_differe

r315251 - [Modules TS] Module ownership semantics for redeclarations.

2017-10-09 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Mon Oct  9 16:42:09 2017
New Revision: 315251

URL: http://llvm.org/viewvc/llvm-project?rev=315251&view=rev
Log:
[Modules TS] Module ownership semantics for redeclarations.

When declaring an entity in the "purview" of a module, it's never a
redeclaration of an entity in the purview of a default module or in no module
("in the global module"). Don't consider those other declarations as possible
redeclaration targets if they're not visible, and reject any cases where we
pick a prior visible declaration that violates this rule.

Added:
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/global-vs-module.cpp
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-global.cpp
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Lookup.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/SemaCXX/modules-ts.cppm

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=315251&r1=315250&r2=315251&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Mon Oct  9 16:42:09 2017
@@ -339,6 +339,12 @@ public:
 return clang::isExternallyVisible(getLinkageInternal());
   }
 
+  /// Determine whether this declaration can be redeclared in a
+  /// different translation unit.
+  bool isExternallyDeclarable() const {
+return isExternallyVisible() && !getOwningModuleForLinkage();
+  }
+
   /// \brief Determines the visibility of this entity.
   Visibility getVisibility() const {
 return getLinkageAndVisibility().getVisibility();
@@ -379,10 +385,6 @@ public:
 return hasCachedLinkage();
   }
 
-  /// Get the module that owns this declaration for linkage purposes.
-  /// There only ever is such a module under the C++ Modules TS.
-  Module *getOwningModuleForLinkage() const;
-
   /// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
   /// the underlying named decl.
   NamedDecl *getUnderlyingDecl() {

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=315251&r1=315250&r2=315251&view=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Mon Oct  9 16:42:09 2017
@@ -738,6 +738,10 @@ public:
 return isFromASTFile() ? getImportedOwningModule() : 
getLocalOwningModule();
   }
 
+  /// Get the module that owns this declaration for linkage purposes.
+  /// There only ever is such a module under the C++ Modules TS.
+  Module *getOwningModuleForLinkage() const;
+
   /// \brief Determine whether this declaration might be hidden from name
   /// lookup. Note that the declaration might be visible even if this returns
   /// \c false, if the owning module is visible within the query context.

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=315251&r1=315250&r2=315251&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Oct  9 16:42:09 
2017
@@ -4801,6 +4801,9 @@ def err_thread_non_thread : Error<
 def err_thread_thread_different_kind : Error<
   "thread-local declaration of %0 with %select{static|dynamic}1 initialization 
"
   "follows declaration with %select{dynamic|static}1 initialization">;
+def err_mismatched_owning_module : Error<
+  "declaration of %0 in %select{the global module|module %2}1 follows "
+  "declaration in %select{the global module|module %4}3">;
 def err_redefinition_different_type : Error<
   "redefinition of %0 with a different type%diff{: $ vs $|}1,2">;
 def err_redefinition_different_kind : Error<

Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=315251&r1=315250&r2=315251&view=diff
==
--- cfe/trunk/include/clang/Sema/Lookup.h (original