There are some comments on the code for this specific patch, which I think
covers the right first chunk of functionality.
That said, I'd like to understand better where you're going with this
feature. Specifically, is the intent that all declarations that come from
private headers be marked "__module_private__", i.e., only accessible within
the module itself?
================
Comment at: include/clang/Basic/DiagnosticLexKinds.td:583
@@ -582,1 +582,3 @@
+def error_use_of_private_header_outside_module : Error<
+ "use of private header from outside its module">;
----------------
How about including the header name in the diagnostic?
================
Comment at: include/clang/Lex/ModuleMap.h:316
@@ +315,3 @@
+ /// (PrivateHeader) This header is included but private.
+ /// (ExcludedHeader) This header is explicitly excluded from the module.
+ void addHeader(Module *Mod, const FileEntry *Header,
----------------
I think the NormalHeader/PrivateHeader/ExcludedHeader documentation should go
on the enumerators up in the enum declaration, because these values aren't
specific to the addHeader function.
================
Comment at: lib/Lex/ModuleMap.cpp:571
@@ -569,2 +570,3 @@
+ Mod->PrivateHeaders.push_back(Header);
Mod->Headers.push_back(Header);
HeaderInfo.MarkFileModuleHeader(Header);
----------------
This puts private headers into both PrivateHeaders and Headers; I don't think
that makes sense. Missing "else"?
================
Comment at: lib/Lex/ModuleMap.cpp:574
@@ -571,3 +573,3 @@
}
- Headers[Header] = KnownHeader(Mod, Excluded);
+ Headers[Header] = KnownHeader(Mod, Role == ExcludedHeader);
}
----------------
I think you'll want to extend KnownHeader to store a ModuleHeaderRole, rather
than just a bool. It'll be useful later on... (see comment below).
================
Comment at: lib/Lex/PPDirectives.cpp:556
@@ +555,3 @@
+ // not enough information is coming back with the module.
+ SmallVectorImpl<const FileEntry *> &PvtHdrs
+ = RequestedModule->PrivateHeaders;
----------------
One way to address this inefficiency is to SuggestedModule from a Module** to a
KnownHeader*, which will have information about both the module and the how
this header is a part of the module. Of course, both findModuleForHeader()'s
will need to be updated accordingly, but that'll propagate the information back
efficiently.
================
Comment at: lib/Lex/PPDirectives.cpp:563
@@ +562,3 @@
+ Module *IncludingModule = HeaderInfo.findModuleForHeader(CurFileEnt);
+ if (IncludingModule != *SuggestedModule)
+ Diag(FilenameLoc,
diag::error_use_of_private_header_outside_module);
----------------
A better way to check this is to check whether
(*SuggestedModule)->getTopLevelModule() == getCurrentModule()
================
Comment at: lib/Serialization/ASTReader.cpp:1290
@@ -1289,2 +1289,3 @@
Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
- ModMap.addHeader(Mod, FileMgr.getFile(key.Filename), /*Excluded=*/false);
+ // FIXME: The following call fails to account for private headers.
+ ModMap.addHeader(Mod, FileMgr.getFile(key.Filename),
----------------
Right. To handle this appropriately, you'll need to serialize another bit into
the flags byte indicating whether this is a normal header vs. a private header.
http://llvm-reviews.chandlerc.com/D584
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
