[PATCH] D52973: Add type_info predefined decl
mwasplund abandoned this revision. mwasplund added a comment. I completely forgot this was open. I have been making incremental improvements to modules ts and this has gotten pulled into that change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52973/new/ https://reviews.llvm.org/D52973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52973: Add type_info predefined decl
mwasplund added inline comments. Comment at: lib/Sema/SemaDecl.cpp:1467-1470 + // FIXME: The Modules TS does not specify how to handle inplicit types + // For now we will simply ignore the implicit global types + if (Old->isImplicit()) +return false; rsmith wrote: > Rather than doing this, please change `ASTContext::buildImplicitRecord` to > `setModuleOwnershipKind(Decl::ModuleOwnershipKind::Unowned)` on the created > declaration. That doesn't seem to work. The check still fails since the owning module is now null. I can fix that by ignoring decls that have no owner on the old version, but I am also running into issues where the cached linkage no longer matches up. Repository: rC Clang https://reviews.llvm.org/D52973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52973: Add type_info predefined decl
mwasplund added a comment. Can you see the full change? I am only seeing that latest commit after running: "git show HEAD -U99 > mypatch.patch". It was working fine with I ran "git diff master..mybranch > mybranch.diff" however that does not give the full context. Repository: rC Clang https://reviews.llvm.org/D52973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52973: Add type_info predefined decl
mwasplund added inline comments. Comment at: lib/Sema/SemaDecl.cpp:1464 if (NewM == OldM) return false; rsmith wrote: > Somewhere up here we're calling `getOwningModule()` but should be calling > `getOwningModuleForLinkage()`. (Please upload patches with full context as > described at https://llvm.org/docs/Phabricator.html) I am using the git mirror to work out of so I can commit incremental changes. However when I used the commands provided they were only showing my latest change, not the full diff from origin master. I will give it another try... Repository: rC Clang https://reviews.llvm.org/D52973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52973: Add type_info predefined decl
mwasplund updated this revision to Diff 169879.
mwasplund marked 2 inline comments as done.
Repository:
rC Clang
https://reviews.llvm.org/D52973
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
lib/Sema/SemaDecl.cpp
test/Modules/msvc-compat-implitic-types.cpp
Index: test/Modules/msvc-compat-implitic-types.cpp
===
--- /dev/null
+++ test/Modules/msvc-compat-implitic-types.cpp
@@ -0,0 +1,14 @@
+// Verify there is not collision between the
+// RUN: %clang -fms-compatibility -fmodules-ts -x c++-module --precompile
-DEXPORT -o %t.pcm %s
+// RUN: %clang -fms-compatibility -fmodules-ts -fmodule-file=%t.pcm -c -DMAIN
%s
+
+// expected-no-diagnostics
+
+#if EXPORT
+export module MyModule;
+export class MyClass {};
+#elif MAIN
+#include
+#else
+#error MISSING DEFINE
+#endif
\ No newline at end of file
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -1464,11 +1464,6 @@
if (NewM == OldM)
return false;
- // FIXME: The Modules TS does not specify how to handle inplicit types
- // For now we will simply ignore the implicit global types
- if (Old->isImplicit())
-return false;
-
// FIXME: Check proclaimed-ownership-declarations here too.
bool NewIsModuleInterface = NewM && NewM->Kind ==
Module::ModuleInterfaceUnit;
bool OldIsModuleInterface = OldM && OldM->Kind ==
Module::ModuleInterfaceUnit;
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1081,8 +1081,10 @@
}
RecordDecl *ASTContext::getTypeInfoClassDecl() const {
- if (!TypeInfoClassDecl)
+ if (!TypeInfoClassDecl) {
TypeInfoClassDecl = buildImplicitRecord("type_info", TTK_Class);
+
TypeInfoClassDecl->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Unowned);
+ }
return TypeInfoClassDecl;
}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1126,7 +1126,7 @@
/// Retrieve the declaration for the 128-bit unsigned integer type.
TypedefDecl *getUInt128Decl() const;
- /// Retrieve the declaration for the type_info class type.
+ /// Retrieve the declaration for the MSVC ::type_info class type.
RecordDecl *getTypeInfoClassDecl() const;
//======//
Index: test/Modules/msvc-compat-implitic-types.cpp
===
--- /dev/null
+++ test/Modules/msvc-compat-implitic-types.cpp
@@ -0,0 +1,14 @@
+// Verify there is not collision between the
+// RUN: %clang -fms-compatibility -fmodules-ts -x c++-module --precompile -DEXPORT -o %t.pcm %s
+// RUN: %clang -fms-compatibility -fmodules-ts -fmodule-file=%t.pcm -c -DMAIN %s
+
+// expected-no-diagnostics
+
+#if EXPORT
+export module MyModule;
+export class MyClass {};
+#elif MAIN
+#include
+#else
+#error MISSING DEFINE
+#endif
\ No newline at end of file
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -1464,11 +1464,6 @@
if (NewM == OldM)
return false;
- // FIXME: The Modules TS does not specify how to handle inplicit types
- // For now we will simply ignore the implicit global types
- if (Old->isImplicit())
-return false;
-
// FIXME: Check proclaimed-ownership-declarations here too.
bool NewIsModuleInterface = NewM && NewM->Kind == Module::ModuleInterfaceUnit;
bool OldIsModuleInterface = OldM && OldM->Kind == Module::ModuleInterfaceUnit;
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1081,8 +1081,10 @@
}
RecordDecl *ASTContext::getTypeInfoClassDecl() const {
- if (!TypeInfoClassDecl)
+ if (!TypeInfoClassDecl) {
TypeInfoClassDecl = buildImplicitRecord("type_info", TTK_Class);
+TypeInfoClassDecl->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Unowned);
+ }
return TypeInfoClassDecl;
}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1126,7 +1126,7 @@
/// Retrieve the declaration for the 128-bit unsigned integer type.
TypedefDecl *getUInt128Decl() const;
- /// Retrieve the declaration for the type_info class type.
+ /// Retrieve the declaration for the MSVC ::type_info class type.
RecordDecl *getTypeInfoClassDecl() const;
//======//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[PATCH] D52973: Add type_info predefined decl
mwasplund added a comment. In https://reviews.llvm.org/D52973#1259909, @rsmith wrote: > Generally this looks good, but it needs an accompanying test. I was looking into that and trying to read up on the documentation for adding tests. I think I understand the crazy comment RUN test invocation, but got lost in the directory structure. Is there documentation on naming and placing regression tests? At first glance it seems arbitrary to me if the file is something like p6.cpp (which don't appear to be sequential) or my-test-name.cpp. Repository: rC Clang https://reviews.llvm.org/D52973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52973: Add type_info predefined decl
mwasplund created this revision.
Herald added a subscriber: cfe-commits.
Bug 39052 - [Modules TS] MSVC std library produces ambiguous type_info
reference when including module with ms-compatibility
When compiling a modules-ts project with MSVC compatibility turned on the
type_info class was getting defined multiple times. With this change I am
adding the type_info class as a predefined decl to allow the reader/writer to
resolve the duplicated declarations.
On top of this I am adding an extra check to ignore redeclaration checks on
implicit types. This could be up for discussion since the spec does not specify
if the implicit types should be in the global unit or if they can be overridden
by the same type inside a module purview.
Repository:
rC Clang
https://reviews.llvm.org/D52973
Files:
lib/Sema/SemaDecl.cpp
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5235,7 +5235,7 @@
DeclarationName Name,
SourceLocation Loc, bool IsTemplateId)
{
DeclContext *Cur = CurContext;
- while (Cur->isTransparentContext() || isa(Cur))
+ while (isa(Cur) || isa(Cur))
Cur = Cur->getParent();
// If the user provided a superfluous scope specifier that refers back to the
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5235,7 +5235,7 @@
DeclarationName Name,
SourceLocation Loc, bool IsTemplateId) {
DeclContext *Cur = CurContext;
- while (Cur->isTransparentContext() || isa(Cur))
+ while (isa(Cur) || isa(Cur))
Cur = Cur->getParent();
// If the user provided a superfluous scope specifier that refers back to the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
