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.

llvm-svn: 189957

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 <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(SmallVectorImpl<std::string> &HeaderFileNames,
   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);
+      HeaderFileName = HeaderDirectory;
+      sys::path::append(HeaderFileName, Line);
     }
-    // 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 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));
 

clang-tools-extra/test/modularize/Inputs/IsDependent.h

@@ -1,4 +0,0 @@
-// This header depends on SomeTypes.h for the TypeInt typedef.
-
-typedef TypeInt NewTypeInt;
-typedef OtherTypeInt OtherNewTypeInt;

clang-tools-extra/test/modularize/Inputs/SomeOtherTypes.h

@@ -1,4 +0,0 @@
-// Declare another type for the dependency check.
-// This file dependent on SomeTypes.h being included first.
-
-typedef TypeInt OtherTypeInt;

clang-tools-extra/test/modularize/NoProblemsDependencies.modularize

@@ -1,6 +0,0 @@
-# RUN: modularize %s -x c++
-
-Inputs/IsDependent.h: Inputs/SomeTypes.h Inputs/SomeOtherTypes.h
-
-# FIXME: Investigating.
-# XFAIL: win32