Hi silvas, klimek,
Here's another crack at it using the options subsystems this time. (Thanks for
the suggestions Sean and Manuel!)
The original summary:
In using modularize to check a large group of platform headers for
modules-readiness, I found that a few headers had dependencies, such that they
required other headers to be included first to avoid compile errors on missing
definitions.
This patch adds support to modularize to allow specifying depended-on headers
in the header file list input to modularize, i.e.
header.h: dependency1.h dependency2.h
http://llvm-reviews.chandlerc.com/D1474
Files:
test/modularize/NoProblemsDependencies.modularize
test/modularize/Inputs/SomeOtherTypes.h
test/modularize/Inputs/IsDependent.h
modularize/Modularize.cpp
modularize/CMakeLists.txt
Index: test/modularize/NoProblemsDependencies.modularize
===================================================================
--- test/modularize/NoProblemsDependencies.modularize
+++ test/modularize/NoProblemsDependencies.modularize
@@ -0,0 +1,3 @@
+# RUN: modularize %s -x c++
+
+Inputs/IsDependent.h: Inputs/SomeTypes.h Inputs/SomeOtherTypes.h
Index: test/modularize/Inputs/SomeOtherTypes.h
===================================================================
--- test/modularize/Inputs/SomeOtherTypes.h
+++ test/modularize/Inputs/SomeOtherTypes.h
@@ -0,0 +1,4 @@
+// Declare another type for the dependency check.
+// This file dependent on SomeTypes.h being included first.
+
+typedef TypeInt OtherTypeInt;
Index: test/modularize/Inputs/IsDependent.h
===================================================================
--- test/modularize/Inputs/IsDependent.h
+++ test/modularize/Inputs/IsDependent.h
@@ -0,0 +1,4 @@
+// This header depends on SomeTypes.h for the TypeInt typedef.
+
+typedef TypeInt NewTypeInt;
+typedef OtherTypeInt OtherNewTypeInt;
Index: modularize/Modularize.cpp
===================================================================
--- modularize/Modularize.cpp
+++ modularize/Modularize.cpp
@@ -18,6 +18,11 @@
// Modularize takes as argument a file name for a file containing the
// newline-separated list of headers to check with respect to each other.
// Lines beginning with '#' and empty lines are ignored.
+// Header file names followed by a colon and other space-separated
+// file names will include those extra files as dependencies.
+// The file names can be relative or full paths, but must be on the
+// same line.
+//
// Modularize also accepts regular front-end arguments.
//
// Usage: modularize [-prefix (optional header path prefix)]
@@ -113,12 +118,7 @@
// 4. Check for correct and consistent usage of extern "C" {} and other
// directives. Warn about #include inside extern "C" {}.
//
-// 5. To support headers that depend on other headers to be included first
-// add support for a dependency list to the header list input.
-// I.e.: header.h: dependent1.h dependent2.h
-// (Implement using clang's "-include" option"?)
-//
-// 6. What else?
+// 5. What else?
//
// General clean-up and refactoring:
//
@@ -132,14 +132,20 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Driver/Options.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/OwningPtr.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/Config/config.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/OptTable.h"
+#include "llvm/Option/Option.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -151,9 +157,12 @@
#include <vector>
#include "PreprocessorTracker.h"
-using namespace clang::tooling;
using namespace clang;
+using namespace clang::driver;
+using namespace clang::driver::options;
+using namespace clang::tooling;
using namespace llvm;
+using namespace llvm::opt;
using namespace Modularize;
// Option to specify a file name for a list of header files to check.
@@ -174,13 +183,20 @@
" If not specified,"
" the files are considered to be relative to the header list file."));
-// Read the header list file and collect the header file names.
-error_code getHeaderFileNames(SmallVectorImpl<std::string> &HeaderFileNames,
- StringRef ListFileName, StringRef HeaderPrefix) {
+typedef SmallVector<std::string, 4> DependentsVector;
+typedef StringMap<DependentsVector> DependencyMap;
+// Read the header list file and collect the header file names and
+// optional dependencies.
+error_code getHeaderFileNames(SmallVectorImpl<std::string> &HeaderFileNames,
+ DependencyMap &Dependencies,
+ StringRef ListFileName,
+ StringRef HeaderPrefix) {
// By default, use the path component of the list file name.
SmallString<256> HeaderDirectory(ListFileName);
sys::path::remove_filename(HeaderDirectory);
+ SmallString<256> CurrentDirectory;
+ sys::fs::current_path(CurrentDirectory);
// Get the prefix if we have one.
if (HeaderPrefix.size() != 0)
@@ -200,25 +216,93 @@
for (SmallVectorImpl<StringRef>::iterator I = Strings.begin(),
E = Strings.end();
I != E; ++I) {
- StringRef Line = (*I).trim();
+ StringRef Line = I->trim();
// Ignore comments and empty lines.
if (Line.empty() || (Line[0] == '#'))
continue;
+ std::pair<StringRef, StringRef> TargetAndDependents = Line.split(':');
SmallString<256> HeaderFileName;
// Prepend header file name prefix if it's not absolute.
- if (sys::path::is_absolute(Line))
- HeaderFileName = Line;
+ if (sys::path::is_absolute(TargetAndDependents.first))
+ llvm::sys::path::native(TargetAndDependents.first, HeaderFileName);
else {
- HeaderFileName = HeaderDirectory;
- sys::path::append(HeaderFileName, Line);
+ if (HeaderDirectory.size() != 0)
+ HeaderFileName = HeaderDirectory;
+ else
+ HeaderFileName = CurrentDirectory;
+ sys::path::append(HeaderFileName, TargetAndDependents.first);
+ llvm::sys::path::native(HeaderFileName.str(), HeaderFileName);
+ }
+ // Handle optional dependencies.
+ DependentsVector Dependents;
+ SmallVector<StringRef, 4> DependentsList;
+ TargetAndDependents.second.split(DependentsList, " ", -1, false);
+ int Count = DependentsList.size();
+ for (int Index = 0; Index < Count; ++Index) {
+ SmallString<256> Dependent;
+ if (sys::path::is_absolute(DependentsList[Index]))
+ Dependent = DependentsList[Index];
+ else {
+ if (HeaderDirectory.size() != 0)
+ Dependent = HeaderDirectory;
+ else
+ Dependent = CurrentDirectory;
+ sys::path::append(Dependent, DependentsList[Index]);
+ }
+ llvm::sys::path::native(Dependent.str(), Dependent);
+ Dependents.push_back(Dependent.str());
}
- // Save the resulting header file path.
+ // Save the resulting header file path and dependencies.
HeaderFileNames.push_back(HeaderFileName.str());
+ Dependencies[HeaderFileName.str()] = Dependents;
}
return error_code::success();
}
+// We provide this derivation to add in "-include (file)" arguments for header
+// dependencies.
+class AddDependenciesAdjuster : public ArgumentsAdjuster {
+public:
+ AddDependenciesAdjuster(DependencyMap &Dependencies)
+ : Dependencies(Dependencies) {}
+private:
+ // Callback for adjusting commandline arguments.
+ CommandLineArguments Adjust(const CommandLineArguments &Args) {
+ llvm::StringRef InputFile = findInputFile(Args);
+ DependentsVector &FileDependents = Dependencies[InputFile];
+ int Count = FileDependents.size();
+ if (Count == 0)
+ return Args;
+ CommandLineArguments NewArgs(Args);
+ for (int Index = 0; Index < Count; ++Index) {
+ NewArgs.push_back("-include");
+ std::string File(
+ std::string("\"") + FileDependents[Index] + std::string("\""));
+ NewArgs.push_back(FileDependents[Index]);
+ }
+ return NewArgs;
+ }
+ // Helper function for finding the input file in an arguments list.
+ llvm::StringRef findInputFile(const CommandLineArguments &CLArgs) {
+ OwningPtr<OptTable> Opts(createDriverOptTable());
+ const unsigned IncludedFlagsBitmask = options::CC1Option;
+ unsigned MissingArgIndex, MissingArgCount;
+ SmallVector<const char*, 256> Argv;
+ for (CommandLineArguments::const_iterator I = CLArgs.begin(),
+ E = CLArgs.end();
+ I != E; ++I)
+ Argv.push_back(I->c_str());
+ OwningPtr<InputArgList> Args(
+ Opts->ParseArgs(Argv.data(), Argv.data() + Argv.size(),
+ MissingArgIndex, MissingArgCount,
+ IncludedFlagsBitmask));
+ std::vector<std::string> Inputs = Args->getAllArgValues(OPT_INPUT);
+ return Inputs.back();
+ }
+ DependencyMap &Dependencies;
+};
+
// FIXME: The Location class seems to be something that we might
// want to design to be applicable to a wider range of tools, and stick it
// somewhere into Tooling/ in mainline
@@ -524,14 +608,16 @@
return 1;
}
- // Get header file names.
+ // Get header file names and dependencies.
SmallVector<std::string, 32> Headers;
- if (error_code EC = getHeaderFileNames(Headers, ListFileName, HeaderPrefix)) {
+ DependencyMap Dependencies;
+ if (error_code EC = getHeaderFileNames(
+ Headers, Dependencies, ListFileName, HeaderPrefix)) {
errs() << Argv[0] << ": error: Unable to get header list '" << ListFileName
<< "': " << EC.message() << '\n';
return 1;
}
-
+
// Create the compilation database.
SmallString<256> PathBuf;
sys::fs::current_path(PathBuf);
@@ -545,6 +631,7 @@
// Parse all of the headers, detecting duplicates.
EntityMap Entities;
ClangTool Tool(*Compilations, Headers);
+ Tool.appendArgumentsAdjuster(new AddDependenciesAdjuster(Dependencies));
int HadErrors =
Tool.run(new ModularizeFrontendActionFactory(Entities, *PPTracker));
Index: modularize/CMakeLists.txt
===================================================================
--- modularize/CMakeLists.txt
+++ modularize/CMakeLists.txt
@@ -13,5 +13,6 @@
target_link_libraries(modularize
clangTooling
clangBasic
+ clangDriver
clangRewriteFrontend
)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
