<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">No long term view yet. Having modularize extract the list of headers from the module map is not fully fleshed out yet. The patch with my first cut at using
a module map didn’t try to distinguish between the header in that way. It did do some extrapolation that umbrella headers probably don’t go in the list directly, assuming modularize will see them through an umbrella header. In my specific case of modularizing
a large set of real world headers, I couldn’t think of a clean way for modularize to avoid unwanted warnings on #includes inside extern/namespace/class blocks, so I came up with this as a work-around. It might be that I can flag headers marked that was in
the module map to be ignored in the block check, but I haven’t planned on doing away with the raw header list and using only the module map at this point.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-John<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> cfe-commits-bounces@cs.uiuc.edu [mailto:cfe-commits-bounces@cs.uiuc.edu]
<b>On Behalf Of </b>Sean Silva<br>
<b>Sent:</b> Wednesday, February 11, 2015 4:32 PM<br>
<b>To:</b> John Thompson<br>
<b>Cc:</b> cfe-commits@cs.uiuc.edu<br>
<b>Subject:</b> Re: [clang-tools-extra] r228846 - Added -block-check-header-list-only option. This is a work-around for private includes that purposefully get included inside blocks.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">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.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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).<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-- Sean Silva<o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Wed, Feb 11, 2015 at 8:58 AM, John Thompson <<a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">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><o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>