diff --git a/clang-tools-extra/modularize/Modularize.cpp b/clang-tools-extra/modularize/Modularize.cpp index 63a4a7073925..8b68e488ab72 100644 --- a/clang-tools-extra/modularize/Modularize.cpp +++ b/clang-tools-extra/modularize/Modularize.cpp @@ -18,11 +18,6 @@ // 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)] @@ -77,29 +72,6 @@ // // See PreprocessorTracker.cpp for additional details. // -// Current problems: -// -// Modularize has problems with C++: -// -// 1. Modularize doesn't distinguish class of the same name in -// different namespaces. The result is erroneous duplicate definition errors. -// -// 2. Modularize doesn't distinguish between a regular class and a template -// class of the same name. -// -// Other problems: -// -// 3. There seem to be a lot of spurious "not always provided" messages, -// and many duplicates of these. -// -// 4. There are some legitimate uses of preprocessor macros that -// modularize will flag as errors, such as repeatedly #include'ing -// a file and using interleaving defined/undefined macros -// to change declarations in the included file. Is there a way -// to address this? Maybe have modularize accept a list of macros -// to ignore. Otherwise you can just exclude the file, after checking -// for legitimate errors. -// // Future directions: // // Basically, we want to add new checks for whatever we can check with respect @@ -107,18 +79,30 @@ // // Some ideas: // -// 1. Fix the C++ and other problems. -// -// 2. Add options to disable any of the checks, in case +// 1. Add options to disable any of the checks, in case // there is some problem with them, or the messages get too verbose. // -// 3. Try to figure out the preprocessor conditional directives that +// 2. Try to figure out the preprocessor conditional directives that // contribute to problems and tie them to the inconsistent definitions. // -// 4. Check for correct and consistent usage of extern "C" {} and other +// 3. Check for correct and consistent usage of extern "C" {} and other // directives. Warn about #include inside extern "C" {}. // -// 5. What else? +// 4. There seem to be a lot of spurious "not always provided" messages, +// and many duplicates of these, which seem to occur when something is +// defined within a preprocessor conditional block, even if the conditional +// always evaluates the same in the stand-alone case. Perhaps we could +// collapse the duplicates, and add an option for disabling the test (see #4). +// +// 5. There are some legitimate uses of preprocessor macros that +// modularize will flag as errors, such as repeatedly #include'ing +// a file and using interleaving defined/undefined macros +// to change declarations in the included file. Is there a way +// to address this? Maybe have modularize accept a list of macros +// to ignore. Otherwise you can just exclude the file, after checking +// for legitimate errors. +// +// 6. What else? // // General clean-up and refactoring: // @@ -132,7 +116,6 @@ #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" @@ -140,12 +123,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/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" @@ -157,12 +135,9 @@ #include #include "PreprocessorTracker.h" -using namespace clang; -using namespace clang::driver; -using namespace clang::driver::options; using namespace clang::tooling; +using namespace clang; using namespace llvm; -using namespace llvm::opt; using namespace Modularize; // Option to specify a file name for a list of header files to check. @@ -183,19 +158,13 @@ cl::opt HeaderPrefix( " If not specified," " the files are considered to be relative to the header list file.")); -typedef SmallVector DependentsVector; -typedef StringMap DependencyMap; - -// Read the header list file and collect the header file names and -// optional dependencies. +// Read the header list file and collect the header file names. error_code getHeaderFileNames(SmallVectorImpl &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) @@ -215,94 +184,25 @@ error_code getHeaderFileNames(SmallVectorImpl &HeaderFileNames, for (SmallVectorImpl::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 TargetAndDependents = Line.split(':'); SmallString<256> HeaderFileName; // Prepend header file name prefix if it's not absolute. - if (sys::path::is_absolute(TargetAndDependents.first)) - llvm::sys::path::native(TargetAndDependents.first, HeaderFileName); + if (sys::path::is_absolute(Line)) + HeaderFileName = Line; else { - if (HeaderDirectory.size() != 0) - HeaderFileName = HeaderDirectory; - else - HeaderFileName = CurrentDirectory; - sys::path::append(HeaderFileName, TargetAndDependents.first); - llvm::sys::path::native(HeaderFileName.str(), HeaderFileName); + HeaderFileName = HeaderDirectory; + sys::path::append(HeaderFileName, Line); } - // Handle optional dependencies. - DependentsVector Dependents; - SmallVector 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 and dependencies. + // Save the resulting header file path. HeaderFileNames.push_back(HeaderFileName.str()); - Dependencies[HeaderFileName.str()] = Dependents; } return error_code::success(); } -// Helper function for finding the input file in an arguments list. -llvm::StringRef findInputFile(const CommandLineArguments &CLArgs) { - OwningPtr Opts(createDriverOptTable()); - const unsigned IncludedFlagsBitmask = options::CC1Option; - unsigned MissingArgIndex, MissingArgCount; - SmallVector Argv; - for (CommandLineArguments::const_iterator I = CLArgs.begin(), - E = CLArgs.end(); - I != E; ++I) - Argv.push_back(I->c_str()); - OwningPtr Args( - Opts->ParseArgs(Argv.data(), Argv.data() + Argv.size(), MissingArgIndex, - MissingArgCount, IncludedFlagsBitmask)); - std::vector Inputs = Args->getAllArgValues(OPT_INPUT); - return Inputs.back(); -} - -// 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; - } - 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 @@ -617,11 +517,9 @@ int main(int Argc, const char **Argv) { return 1; } - // Get header file names and dependencies. + // Get header file names. SmallVector Headers; - DependencyMap Dependencies; - if (error_code EC = getHeaderFileNames(Headers, Dependencies, ListFileName, - HeaderPrefix)) { + if (error_code EC = getHeaderFileNames(Headers, ListFileName, HeaderPrefix)) { errs() << Argv[0] << ": error: Unable to get header list '" << ListFileName << "': " << EC.message() << '\n'; return 1; @@ -640,7 +538,6 @@ int main(int Argc, const char **Argv) { // 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)); diff --git a/clang-tools-extra/test/modularize/Inputs/IsDependent.h b/clang-tools-extra/test/modularize/Inputs/IsDependent.h deleted file mode 100644 index 492811026603..000000000000 --- a/clang-tools-extra/test/modularize/Inputs/IsDependent.h +++ /dev/null @@ -1,4 +0,0 @@ -// This header depends on SomeTypes.h for the TypeInt typedef. - -typedef TypeInt NewTypeInt; -typedef OtherTypeInt OtherNewTypeInt; diff --git a/clang-tools-extra/test/modularize/Inputs/SomeOtherTypes.h b/clang-tools-extra/test/modularize/Inputs/SomeOtherTypes.h deleted file mode 100644 index 288faffbb023..000000000000 --- a/clang-tools-extra/test/modularize/Inputs/SomeOtherTypes.h +++ /dev/null @@ -1,4 +0,0 @@ -// Declare another type for the dependency check. -// This file dependent on SomeTypes.h being included first. - -typedef TypeInt OtherTypeInt; diff --git a/clang-tools-extra/test/modularize/NoProblemsDependencies.modularize b/clang-tools-extra/test/modularize/NoProblemsDependencies.modularize deleted file mode 100644 index db4a6edd2e48..000000000000 --- a/clang-tools-extra/test/modularize/NoProblemsDependencies.modularize +++ /dev/null @@ -1,6 +0,0 @@ -# RUN: modularize %s -x c++ - -Inputs/IsDependent.h: Inputs/SomeTypes.h Inputs/SomeOtherTypes.h - -# FIXME: Investigating. -# XFAIL: win32