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 < [email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
