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/D1383
Files:
test/modularize/NoProblemsDependencies.modularize
test/modularize/Inputs/SomeOtherTypes.h
test/modularize/Inputs/IsDependent.h
modularize/Modularize.cpp
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:
//
@@ -139,6 +139,7 @@
#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/Support/CommandLine.h"
#include "llvm/Support/FileSystem.h"
@@ -174,13 +175,19 @@
" If not specified,"
" the files are considered to be relative to the header list file."));
+typedef SmallVector<std::string, 4> DependentsVector;
+typedef StringMap<DependentsVector> DependencyMap;
+
// Read the header list file and collect the header file names.
error_code getHeaderFileNames(SmallVectorImpl<std::string> &HeaderFileNames,
- StringRef ListFileName, StringRef HeaderPrefix) {
-
+ 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 +207,151 @@
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.
+ // FIXME: This should really be in the tooling library, or even
+ // better, have the Adjust function be given an InputFile argument.
+ // But since I couldn't get that change through, we're stuck
+ // with this mechanism, which has the unfortunate disadvantage
+ // of having to know which compile options take arguments.
+ llvm::StringRef findInputFile(const CommandLineArguments &Args) {
+ llvm::StringRef InputFile;
+ CommandLineArguments::const_iterator E = Args.end();
+ CommandLineArguments::const_iterator I = Args.begin();
+ // Walk arguments, skipping the first, "clang_tool".
+ for (++I; I != E; ++I) {
+ llvm::StringRef Arg = *I;
+ if (Arg[0] != '-') {
+ InputFile = Arg;
+ break;
+ }
+ else if (isOptionWithArgument(Arg))
+ ++I; // Skip option with argument to avoid false input file detection.
+ }
+ return InputFile;
+ }
+ // Helper function to check for an option that has an argument.
+ bool isOptionWithArgument(llvm::StringRef option) {
+ for (int Index = 0; OptionsWithArgument[Index] != NULL; ++Index) {
+ if (option.compare(OptionsWithArgument[Index]) == 0)
+ return true;
+ }
+ return false;
+ }
+ DependencyMap &Dependencies;
+ static const char *OptionsWithArgument[];
+};
+
+// The most common (but not all) options to modularize that take
+// an argument in the following slot.
+// Note: This needs to be kept in sync with new or removed Clang arguments.
+// But hopefully modularize users won't need too many of these
+// kinds of arguments.
+const char *AddDependenciesAdjuster::OptionsWithArgument[] = {
+ "-D",
+ "-F",
+ "-I",
+ "-MQ",
+ "-MT",
+ "-Wa",
+ "-Wl",
+ "-Wp",
+ "-Xanalyzer",
+ "-Xassembler",
+ "-Xclang",
+ "-Xlinker",
+ "-Xpreprocessor",
+ "-arcmt-migrate-report-output",
+ "-cxx-isystem",
+ "-dependency-dot",
+ "-dependency-file",
+ "-idirafter",
+ "-iframework",
+ "-imacros",
+ "-include-pch",
+ "-include",
+ "-iprefix",
+ "-iquote",
+ "-isysroot",
+ "-isystem",
+ "-iwithprefixbefore",
+ "-iwithprefix",
+ "-iwithsysroot",
+ "-mllvm",
+ "-o",
+ "-target",
+ "-triple",
+ "-working-directory",
+ "-x",
+ NULL
+};
+
// 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 +657,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 +680,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));
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
