rsmith added a comment.
Generally this looks good, but it needs an accompanying test.
================
Comment at: include/clang/AST/ASTContext.h:1129-1130
+ /// Retrieve the declaration for the type_info class type.
+ RecordDecl *getTypeInfoClassDecl() const;
+
----------------
Please indicate somewhere that this is the non-standard MSVC `::type_info`
type, not the standard `std::type_info` type.
================
Comment at: lib/Sema/SemaDecl.cpp:1464
if (NewM == OldM)
return false;
----------------
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)
================
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;
----------------
Rather than doing this, please change `ASTContext::buildImplicitRecord` to
`setModuleOwnershipKind(Decl::ModuleOwnershipKind::Unowned)` on the created
declaration.
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
