[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.

Thompson, John John_Thompson at playstation.sony.com
Wed Feb 11 17:08:24 PST 2015


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.

-John

From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Sean Silva
Sent: Wednesday, February 11, 2015 4:32 PM
To: John Thompson
Cc: cfe-commits at cs.uiuc.edu
Subject: 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.

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.

We could use the `textual` specifier (see http://clang.llvm.org/docs/Modules.html#header-declaration) 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).

-- Sean Silva

On Wed, Feb 11, 2015 at 8:58 AM, John Thompson <John.Thompson.JTSoftware at gmail.com<mailto:John.Thompson.JTSoftware at gmail.com>> wrote:
Author: jtsoftware
Date: Wed Feb 11 10:58:36 2015
New Revision: 228846

URL: http://llvm.org/viewvc/llvm-project?rev=228846&view=rev
Log:
Added -block-check-header-list-only option.  This is a work-around for private includes that purposefully get included inside blocks.

Added:
    clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize
Modified:
    clang-tools-extra/trunk/docs/ModularizeUsage.rst
    clang-tools-extra/trunk/modularize/Modularize.cpp
    clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp
    clang-tools-extra/trunk/modularize/PreprocessorTracker.h

Modified: clang-tools-extra/trunk/docs/ModularizeUsage.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ModularizeUsage.rst?rev=228846&r1=228845&r2=228846&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ModularizeUsage.rst (original)
+++ clang-tools-extra/trunk/docs/ModularizeUsage.rst Wed Feb 11 10:58:36 2015
@@ -49,3 +49,10 @@ Modularize Command Line Options

   Put modules generated by the -module-map-path option in an enclosing
   module with the given name.  See the description in :ref:`module-map-generation`.
+
+.. option:: -block-check-header-list-only
+
+  Limit the #include-inside-extern-or-namespace-block
+  check to only those headers explicitly listed in the header list.
+  This is a work-around for avoiding error messages for private includes that
+  purposefully get included inside blocks.

Modified: clang-tools-extra/trunk/modularize/Modularize.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=228846&r1=228845&r2=228846&view=diff
==============================================================================
--- clang-tools-extra/trunk/modularize/Modularize.cpp (original)
+++ clang-tools-extra/trunk/modularize/Modularize.cpp Wed Feb 11 10:58:36 2015
@@ -209,6 +209,15 @@ cl::opt<std::string>
 RootModule("root-module", cl::init(""),
            cl::desc("Specify the name of the root module."));

+// Option for limiting the #include-inside-extern-or-namespace-block
+// check to only those headers explicitly listed in the header list.
+// This is a work-around for private includes that purposefully get
+// included inside blocks.
+static cl::opt<bool>
+BlockCheckHeaderListOnly("block-check-header-list-only", cl::init(false),
+cl::desc("Only warn if #include directives are inside extern or namespace"
+  " blocks if the included header is in the header list."));
+
 // Save the program name for error messages.
 const char *Argv0;
 // Save the command line for comments.
@@ -722,7 +731,8 @@ int main(int Argc, const char **Argv) {
       new FixedCompilationDatabase(Twine(PathBuf), CC1Arguments));

   // Create preprocessor tracker, to watch for macro and conditional problems.
-  std::unique_ptr<PreprocessorTracker> PPTracker(PreprocessorTracker::create());
+  std::unique_ptr<PreprocessorTracker> PPTracker(
+    PreprocessorTracker::create(Headers, BlockCheckHeaderListOnly));

   // Parse all of the headers, detecting duplicates.
   EntityMap Entities;

Modified: clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp?rev=228846&r1=228845&r2=228846&view=diff
==============================================================================
--- clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp (original)
+++ clang-tools-extra/trunk/modularize/PreprocessorTracker.cpp Wed Feb 11 10:58:36 2015
@@ -866,9 +866,19 @@ ConditionalExpansionMapIter;
 // course of running modularize.
 class PreprocessorTrackerImpl : public PreprocessorTracker {
 public:
-  PreprocessorTrackerImpl()
-      : CurrentInclusionPathHandle(InclusionPathHandleInvalid),
-        InNestedHeader(false) {}
+  PreprocessorTrackerImpl(llvm::SmallVector<std::string, 32> &Headers,
+        bool DoBlockCheckHeaderListOnly)
+      : BlockCheckHeaderListOnly(DoBlockCheckHeaderListOnly),
+        CurrentInclusionPathHandle(InclusionPathHandleInvalid),
+        InNestedHeader(false) {
+    // Use canonical header path representation.
+    for (llvm::ArrayRef<std::string>::iterator I = Headers.begin(),
+      E = Headers.end();
+      I != E; ++I) {
+      HeaderList.push_back(getCanonicalPath(*I));
+    }
+  }
+
   ~PreprocessorTrackerImpl() {}

   // Handle entering a preprocessing session.
@@ -889,6 +899,10 @@ public:
   // "namespace {}" blocks containing #include directives.
   void handleIncludeDirective(llvm::StringRef DirectivePath, int DirectiveLine,
                               int DirectiveColumn, llvm::StringRef TargetPath) {
+    // If it's not a header in the header list, ignore it with respect to
+    // the check.
+    if (BlockCheckHeaderListOnly && !isHeaderListHeader(DirectivePath))
+      return;
     HeaderHandle CurrentHeaderHandle = findHeaderHandle(DirectivePath);
     StringHandle IncludeHeaderHandle = addString(TargetPath);
     for (std::vector<PPItemKey>::const_iterator I = IncludeDirectives.begin(),
@@ -959,6 +973,7 @@ public:
     if (!InNestedHeader)
       InNestedHeader = !HeadersInThisCompile.insert(H).second;
   }
+
   // Handle exiting a header source file.
   void handleHeaderExit(llvm::StringRef HeaderPath) {
     // Ignore <built-in> and <command-line> to reduce message clutter.
@@ -982,6 +997,18 @@ public:
     return CanonicalPath;
   }

+  // Return true if the given header is in the header list.
+  bool isHeaderListHeader(llvm::StringRef HeaderPath) const {
+    std::string CanonicalPath = getCanonicalPath(HeaderPath);
+    for (llvm::ArrayRef<std::string>::iterator I = HeaderList.begin(),
+        E = HeaderList.end();
+        I != E; ++I) {
+      if (*I == CanonicalPath)
+        return true;
+    }
+    return false;
+  }
+
   // Get the handle of a header file entry.
   // Return HeaderHandleInvalid if not found.
   HeaderHandle findHeaderHandle(llvm::StringRef HeaderPath) const {
@@ -1301,6 +1328,9 @@ public:
   }

 private:
+  llvm::SmallVector<std::string, 32> HeaderList;
+  // Only do extern, namespace check for headers in HeaderList.
+  bool BlockCheckHeaderListOnly;
   llvm::StringPool Strings;
   std::vector<StringHandle> HeaderPaths;
   std::vector<HeaderHandle> HeaderStack;
@@ -1319,8 +1349,10 @@ private:
 PreprocessorTracker::~PreprocessorTracker() {}

 // Create instance of PreprocessorTracker.
-PreprocessorTracker *PreprocessorTracker::create() {
-  return new PreprocessorTrackerImpl();
+PreprocessorTracker *PreprocessorTracker::create(
+    llvm::SmallVector<std::string, 32> &Headers,
+    bool DoBlockCheckHeaderListOnly) {
+  return new PreprocessorTrackerImpl(Headers, DoBlockCheckHeaderListOnly);
 }

 // Preprocessor callbacks for modularize.

Modified: clang-tools-extra/trunk/modularize/PreprocessorTracker.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/PreprocessorTracker.h?rev=228846&r1=228845&r2=228846&view=diff
==============================================================================
--- clang-tools-extra/trunk/modularize/PreprocessorTracker.h (original)
+++ clang-tools-extra/trunk/modularize/PreprocessorTracker.h Wed Feb 11 10:58:36 2015
@@ -77,7 +77,9 @@ public:
   virtual bool reportInconsistentConditionals(llvm::raw_ostream &OS) = 0;

   // Create instance of PreprocessorTracker.
-  static PreprocessorTracker *create();
+  static PreprocessorTracker *create(
+    llvm::SmallVector<std::string, 32> &Headers,
+    bool DoBlockCheckHeaderListOnly);
 };

 } // end namespace Modularize

Added: clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize?rev=228846&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize (added)
+++ clang-tools-extra/trunk/test/modularize/NoProblemsNamespace.modularize Wed Feb 11 10:58:36 2015
@@ -0,0 +1,3 @@
+# RUN: modularize -block-check-header-list-only
+
+Inputs/IncludeInNamespace.h


_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150212/9118d7d5/attachment.html>


More information about the cfe-commits mailing list