[clang-tools-extra] r189957 - Revert svn 189837 "Added header dependencies support."
Bob Wilson
bob.wilson at apple.com
Wed Sep 4 09:48:28 PDT 2013
Author: bwilson
Date: Wed Sep 4 11:48:28 2013
New Revision: 189957
URL: http://llvm.org/viewvc/llvm-project?rev=189957&view=rev
Log:
Revert svn 189837 "Added header dependencies support."
The NoProblemsDependencies.modularize test is failing on many buildbots.
I have also reverted the change in 189904 to disable that test for MSVC.
Removed:
clang-tools-extra/trunk/test/modularize/Inputs/IsDependent.h
clang-tools-extra/trunk/test/modularize/Inputs/SomeOtherTypes.h
clang-tools-extra/trunk/test/modularize/NoProblemsDependencies.modularize
Modified:
clang-tools-extra/trunk/modularize/Modularize.cpp
Modified: clang-tools-extra/trunk/modularize/Modularize.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=189957&r1=189956&r2=189957&view=diff
==============================================================================
--- clang-tools-extra/trunk/modularize/Modularize.cpp (original)
+++ clang-tools-extra/trunk/modularize/Modularize.cpp Wed Sep 4 11:48:28 2013
@@ -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 <vector>
#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<std::string> HeaderPrefix(
" 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 and
-// optional dependencies.
+// Read the header list file and collect the header file names.
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)
@@ -215,94 +184,25 @@ error_code getHeaderFileNames(SmallVecto
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(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);
- }
- // 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());
+ HeaderFileName = HeaderDirectory;
+ sys::path::append(HeaderFileName, Line);
}
- // 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<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();
-}
-
-// 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<std::string, 32> 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));
Removed: clang-tools-extra/trunk/test/modularize/Inputs/IsDependent.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/IsDependent.h?rev=189956&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/modularize/Inputs/IsDependent.h (original)
+++ clang-tools-extra/trunk/test/modularize/Inputs/IsDependent.h (removed)
@@ -1,4 +0,0 @@
-// This header depends on SomeTypes.h for the TypeInt typedef.
-
-typedef TypeInt NewTypeInt;
-typedef OtherTypeInt OtherNewTypeInt;
Removed: clang-tools-extra/trunk/test/modularize/Inputs/SomeOtherTypes.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/Inputs/SomeOtherTypes.h?rev=189956&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/modularize/Inputs/SomeOtherTypes.h (original)
+++ clang-tools-extra/trunk/test/modularize/Inputs/SomeOtherTypes.h (removed)
@@ -1,4 +0,0 @@
-// Declare another type for the dependency check.
-// This file dependent on SomeTypes.h being included first.
-
-typedef TypeInt OtherTypeInt;
Removed: clang-tools-extra/trunk/test/modularize/NoProblemsDependencies.modularize
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/modularize/NoProblemsDependencies.modularize?rev=189956&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/modularize/NoProblemsDependencies.modularize (original)
+++ clang-tools-extra/trunk/test/modularize/NoProblemsDependencies.modularize (removed)
@@ -1,6 +0,0 @@
-# RUN: modularize %s -x c++
-
-Inputs/IsDependent.h: Inputs/SomeTypes.h Inputs/SomeOtherTypes.h
-
-# FIXME: Investigating.
-# XFAIL: win32
More information about the cfe-commits
mailing list