[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 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 http://lists.llvm.org/cgi-
[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 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: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 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 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
[PATCH] D52973: Add type_info predefined decl
mwasplund updated this revision to Diff 168606. 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
[PATCH] D52973: Add type_info predefined decl
mwasplund updated this revision to Diff 168607. Repository: rC Clang https://reviews.llvm.org/D52973 Files: include/clang/AST/ASTContext.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -4682,7 +4682,9 @@ RegisterPredefDecl(Context.CFConstantStringTagDecl, PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID); RegisterPredefDecl(Context.TypePackElementDecl, - PREDEF_DECL_TYPE_PACK_ELEMENT_ID); + PREDEF_DECL_TYPE_PACK_ELEMENT_ID); + RegisterPredefDecl(Context.TypeInfoClassDecl, + PREDEF_DECL_TYPE_INFO_CLASS_ID); // Build a record containing all of the tentative definitions in this file, in // TentativeDefinitions order. Generally, this record will be empty for Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -7306,6 +7306,9 @@ case PREDEF_DECL_TYPE_PACK_ELEMENT_ID: return Context.getTypePackElementDecl(); + + case PREDEF_DECL_TYPE_INFO_CLASS_ID: +return Context.getTypeInfoClassDecl(); } llvm_unreachable("PredefinedDeclIDs unknown enum value"); } Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -1464,6 +1464,11 @@ 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/Sema/Sema.cpp === --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -247,8 +247,7 @@ if (getLangOpts().MSVCCompat) { if (getLangOpts().CPlusPlus && IdResolver.begin(&Context.Idents.get("type_info")) == IdResolver.end()) - PushOnScopeChains(Context.buildImplicitRecord("type_info", TTK_Class), -TUScope); + PushOnScopeChains(Context.getTypeInfoClassDecl(), TUScope); addImplicitTypedef("size_t", Context.getSizeType()); } Index: lib/AST/ASTContext.cpp === --- lib/AST/ASTContext.cpp +++ lib/AST/ASTContext.cpp @@ -1080,6 +1080,12 @@ return UInt128Decl; } +RecordDecl *ASTContext::getTypeInfoClassDecl() const { + if (!TypeInfoClassDecl) +TypeInfoClassDecl = buildImplicitRecord("type_info", TTK_Class); + return TypeInfoClassDecl; +} + void ASTContext::InitBuiltinType(CanQualType &R, BuiltinType::Kind K) { auto *Ty = new (*this, TypeAlignment) BuiltinType(K); R = CanQualType::CreateUnsafe(QualType(Ty, 0)); Index: include/clang/Serialization/ASTBitCodes.h === --- include/clang/Serialization/ASTBitCodes.h +++ include/clang/Serialization/ASTBitCodes.h @@ -1264,13 +1264,16 @@ /// The internal '__type_pack_element' template. PREDEF_DECL_TYPE_PACK_ELEMENT_ID = 16, + + /// The internal 'type_info' class. + PREDEF_DECL_TYPE_INFO_CLASS_ID = 17, }; /// The number of declaration IDs that are predefined. /// /// For more information about predefined declarations, see the /// \c PredefinedDeclIDs type and the PREDEF_DECL_*_ID constants. -const unsigned int NUM_PREDEF_DECL_IDS = 17; +const unsigned int NUM_PREDEF_DECL_IDS = 18; /// Record of updates for a declaration that was modified after /// being deserialized. This can occur within DECLTYPES_BLOCK_ID. Index: include/clang/AST/ASTContext.h === --- include/clang/AST/ASTContext.h +++ include/clang/AST/ASTContext.h @@ -326,6 +326,9 @@ /// The typedef for the predefined 'BOOL' type. mutable TypedefDecl *BOOLDecl = nullptr; + /// The class for the predifined 'type_info' type. + mutable RecordDecl *TypeInfoClassDecl = nullptr; + // Typedefs which may be provided defining the structure of Objective-C // pseudo-builtins QualType ObjCIdRedefinitionType; @@ -1123,6 +1126,9 @@ /// Retrieve the declaration for the 128-bit unsigned integer type. TypedefDecl *getUInt128Decl() const; + /// Retrieve the declaration for the type_info class type. + RecordDecl *getTypeInfoClassDecl() const; + //===