<div dir="ltr">What is the long-term view you have for this feature? Eventually we want to drive the entire checking process from just the module map.<div><br></div><div>We could use the `textual` specifier (see <a href="http://clang.llvm.org/docs/Modules.html#header-declaration">http://clang.llvm.org/docs/Modules.html#header-declaration</a>) to mark headers that are known to be included in this non-modular fashion and then suppress warnings that come from inclusions of such headers (i.e. we identify the headers that are meant to be included non-modularly, rather than (like in the present patch) files that contain non-modular includes).<div><br></div><div>-- Sean Silva</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 11, 2015 at 8:58 AM, John Thompson <span dir="ltr"><<a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jtsoftware<br>
Date: Wed Feb 11 10:58:36 2015<br>
New Revision: 228846<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=228846&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=228846&view=rev</a><br>
Log:<br>
Added -block-check-header-list-only option. This is a work-around for private includes that purposefully get included inside blocks.<br>
<br>
Added:<br>
clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize<br>
Modified:<br>
clang-tools-extra/trunk/docs/ModularizeUsage.rst<br>
clang-tools-extra/trunk/modularize/Modularize.cpp<br>
clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp<br>
clang-tools-extra/trunk/modularize/PreprocessorTracker.h<br>
<br>
Modified: clang-tools-extra/trunk/docs/ModularizeUsage.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=228846&r1=228845&r2=228846&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=228846&r1=228845&r2=228846&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/docs/ModularizeUsage.rst (original)<br>
+++ clang-tools-extra/trunk/docs/ModularizeUsage.rst Wed Feb 11 10:58:36 2015<br>
@@ -49,3 +49,10 @@ Modularize Command Line Options<br>
<br>
Put modules generated by the -module-map-path option in an enclosing<br>
module with the given name. See the description in :ref:`module-map-generation`.<br>
+<br>
+.. option:: -block-check-header-list-only<br>
+<br>
+ Limit the #include-inside-extern-or-namespace-block<br>
+ check to only those headers explicitly listed in the header list.<br>
+ This is a work-around for avoiding error messages for private includes that<br>
+ purposefully get included inside blocks.<br>
<br>
Modified: clang-tools-extra/trunk/modularize/Modularize.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=228846&r1=228845&r2=228846&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=228846&r1=228845&r2=228846&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/Modularize.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/Modularize.cpp Wed Feb 11 10:58:36 2015<br>
@@ -209,6 +209,15 @@ cl::opt<std::string><br>
RootModule("root-module", cl::init(""),<br>
cl::desc("Specify the name of the root module."));<br>
<br>
+// Option for limiting the #include-inside-extern-or-namespace-block<br>
+// check to only those headers explicitly listed in the header list.<br>
+// This is a work-around for private includes that purposefully get<br>
+// included inside blocks.<br>
+static cl::opt<bool><br>
+BlockCheckHeaderListOnly("block-check-header-list-only", cl::init(false),<br>
+cl::desc("Only warn if #include directives are inside extern or namespace"<br>
+ " blocks if the included header is in the header list."));<br>
+<br>
// Save the program name for error messages.<br>
const char *Argv0;<br>
// Save the command line for comments.<br>
@@ -722,7 +731,8 @@ int main(int Argc, const char **Argv) {<br>
new FixedCompilationDatabase(Twine(PathBuf), CC1Arguments));<br>
<br>
// Create preprocessor tracker, to watch for macro and conditional problems.<br>
- std::unique_ptr<PreprocessorTracker> PPTracker(PreprocessorTracker::create());<br>
+ std::unique_ptr<PreprocessorTracker> PPTracker(<br>
+ PreprocessorTracker::create(Headers, BlockCheckHeaderListOnly));<br>
<br>
// Parse all of the headers, detecting duplicates.<br>
EntityMap Entities;<br>
<br>
Modified: clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp?rev=228846&r1=228845&r2=228846&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp?rev=228846&r1=228845&r2=228846&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp (original)<br>
+++ clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp Wed Feb 11 10:58:36 2015<br>
@@ -866,9 +866,19 @@ ConditionalExpansionMapIter;<br>
// course of running modularize.<br>
class PreprocessorTrackerImpl : public PreprocessorTracker {<br>
public:<br>
- PreprocessorTrackerImpl()<br>
- : CurrentInclusionPathHandle(InclusionPathHandleInvalid),<br>
- InNestedHeader(false) {}<br>
+ PreprocessorTrackerImpl(llvm::SmallVector<std::string, 32> &Headers,<br>
+ bool DoBlockCheckHeaderListOnly)<br>
+ : BlockCheckHeaderListOnly(DoBlockCheckHeaderListOnly),<br>
+ CurrentInclusionPathHandle(InclusionPathHandleInvalid),<br>
+ InNestedHeader(false) {<br>
+ // Use canonical header path representation.<br>
+ for (llvm::ArrayRef<std::string>::iterator I = Headers.begin(),<br>
+ E = Headers.end();<br>
+ I != E; ++I) {<br>
+ HeaderList.push_back(getCanonicalPath(*I));<br>
+ }<br>
+ }<br>
+<br>
~PreprocessorTrackerImpl() {}<br>
<br>
// Handle entering a preprocessing session.<br>
@@ -889,6 +899,10 @@ public:<br>
// "namespace {}" blocks containing #include directives.<br>
void handleIncludeDirective(llvm::StringRef DirectivePath, int DirectiveLine,<br>
int DirectiveColumn, llvm::StringRef TargetPath) {<br>
+ // If it's not a header in the header list, ignore it with respect to<br>
+ // the check.<br>
+ if (BlockCheckHeaderListOnly && !isHeaderListHeader(DirectivePath))<br>
+ return;<br>
HeaderHandle CurrentHeaderHandle = findHeaderHandle(DirectivePath);<br>
StringHandle IncludeHeaderHandle = addString(TargetPath);<br>
for (std::vector<PPItemKey>::const_iterator I = IncludeDirectives.begin(),<br>
@@ -959,6 +973,7 @@ public:<br>
if (!InNestedHeader)<br>
InNestedHeader = !HeadersInThisCompile.insert(H).second;<br>
}<br>
+<br>
// Handle exiting a header source file.<br>
void handleHeaderExit(llvm::StringRef HeaderPath) {<br>
// Ignore <built-in> and <command-line> to reduce message clutter.<br>
@@ -982,6 +997,18 @@ public:<br>
return CanonicalPath;<br>
}<br>
<br>
+ // Return true if the given header is in the header list.<br>
+ bool isHeaderListHeader(llvm::StringRef HeaderPath) const {<br>
+ std::string CanonicalPath = getCanonicalPath(HeaderPath);<br>
+ for (llvm::ArrayRef<std::string>::iterator I = HeaderList.begin(),<br>
+ E = HeaderList.end();<br>
+ I != E; ++I) {<br>
+ if (*I == CanonicalPath)<br>
+ return true;<br>
+ }<br>
+ return false;<br>
+ }<br>
+<br>
// Get the handle of a header file entry.<br>
// Return HeaderHandleInvalid if not found.<br>
HeaderHandle findHeaderHandle(llvm::StringRef HeaderPath) const {<br>
@@ -1301,6 +1328,9 @@ public:<br>
}<br>
<br>
private:<br>
+ llvm::SmallVector<std::string, 32> HeaderList;<br>
+ // Only do extern, namespace check for headers in HeaderList.<br>
+ bool BlockCheckHeaderListOnly;<br>
llvm::StringPool Strings;<br>
std::vector<StringHandle> HeaderPaths;<br>
std::vector<HeaderHandle> HeaderStack;<br>
@@ -1319,8 +1349,10 @@ private:<br>
PreprocessorTracker::~PreprocessorTracker() {}<br>
<br>
// Create instance of PreprocessorTracker.<br>
-PreprocessorTracker *PreprocessorTracker::create() {<br>
- return new PreprocessorTrackerImpl();<br>
+PreprocessorTracker *PreprocessorTracker::create(<br>
+ llvm::SmallVector<std::string, 32> &Headers,<br>
+ bool DoBlockCheckHeaderListOnly) {<br>
+ return new PreprocessorTrackerImpl(Headers, DoBlockCheckHeaderListOnly);<br>
}<br>
<br>
// Preprocessor callbacks for modularize.<br>
<br>
Modified: clang-tools-extra/trunk/modularize/PreprocessorTracker.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.h?rev=228846&r1=228845&r2=228846&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.h?rev=228846&r1=228845&r2=228846&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/modularize/PreprocessorTracker.h (original)<br>
+++ clang-tools-extra/trunk/modularize/PreprocessorTracker.h Wed Feb 11 10:58:36 2015<br>
@@ -77,7 +77,9 @@ public:<br>
virtual bool reportInconsistentConditionals(llvm::raw_ostream &OS) = 0;<br>
<br>
// Create instance of PreprocessorTracker.<br>
- static PreprocessorTracker *create();<br>
+ static PreprocessorTracker *create(<br>
+ llvm::SmallVector<std::string, 32> &Headers,<br>
+ bool DoBlockCheckHeaderListOnly);<br>
};<br>
<br>
} // end namespace Modularize<br>
<br>
Added: clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize?rev=228846&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize?rev=228846&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize (added)<br>
+++ clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize Wed Feb 11 10:58:36 2015<br>
@@ -0,0 +1,3 @@<br>
+# RUN: modularize -block-check-header-list-only<br>
+<br>
+Inputs/IncludeInNamespace.h<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>