[clang-tools-extra] r177959 - Revised to use file list path as reference path, or path provide by new -prefix option. Revised to use LLVM option parser.
Vane, Edwin
edwin.vane at intel.com
Tue Mar 26 10:06:34 PDT 2013
I did an audit in Phabricator but I guess those don't get sent to the cfe-dev list. Here's what I said:
I think I would have liked to see more review on this latest modularize tests patch (I'm referring to modularize_tests_4.patch attached in the thread "modularize, modularize tests - please review"). It's certainly not the obvious fix I think Sean thought you were talking about.
Inline Comments:
/clang-tools-extra/trunk/modularize/Modularize.cpp
355 Could you run clang-format on these lines. Something about them doesn't quite look right. clang-format might not handle the string constants nicely. You may find you have to re-break the lines after the format.
378 LLVM coding style says all new code for reading files should use llvm::MemoryBuffer interface. I don't have much experience with that but there's a general aversion to iostream stuff.
380 .c_str() is not necessary.
400 Redundant use of llvm:: given the using directive.
368 .c_str() not necessary.
21 Should mention that the input file can have lines commented out if character 0 on the line is a '#'.
27 perhaps clearer: "...relative to the directory containing the header list file".
90 Now that you've added this using directive, can you remove all the redundant uses of llvm:: in the code below.
-----Original Message-----
From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of John Thompson
Sent: Monday, March 25, 2013 9:18 PM
To: cfe-commits at cs.uiuc.edu
Subject: [clang-tools-extra] r177959 - Revised to use file list path as reference path, or path provide by new -prefix option. Revised to use LLVM option parser.
Author: jtsoftware
Date: Mon Mar 25 20:17:48 2013
New Revision: 177959
URL: http://llvm.org/viewvc/llvm-project?rev=177959&view=rev
Log:
Revised to use file list path as reference path, or path provide by new -prefix option. Revised to use LLVM option parser.
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=177959&r1=177958&r2=177959&view=diff
==============================================================================
--- clang-tools-extra/trunk/modularize/Modularize.cpp (original)
+++ clang-tools-extra/trunk/modularize/Modularize.cpp Mon Mar 25
+++ 20:17:48 2013
@@ -19,7 +19,13 @@
// newline-separated list of headers to check with respect to each other.
// Modularize also accepts regular front-end arguments.
//
-// Usage: modularize (include-files_list) [(front-end-options) ...]
+// Usage: modularize [-prefix (optional header path prefix)] \
+// (include-files_list) [(front-end-options) ...]
+//
+// Note that unless a "-prefex (header path)" option is specified, //
+non-absolute file paths in the header list file will be relative // to
+the header list file directory. Use -prefix to specify a different //
+directory.
//
// Modularize will do normal parsing, reporting normal errors and warnings, // but will also report special error messages like the following:
@@ -60,7 +66,9 @@
//===----------------------------------------------------------------------===//
#include "llvm/Config/config.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
#include "llvm/ADT/StringRef.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Preprocessor.h"
@@ -79,7 +87,7 @@
using namespace clang::tooling;
using namespace clang;
-using llvm::StringRef;
+using namespace llvm;
// 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 @@ -331,43 +339,68 @@ private:
EntityMap &Entities;
};
+// Option to specify a file name for a list of header files to check.
+cl::opt<std::string> ListFileName(cl::Positional,
+ cl::desc("<name of file containing list of headers to check>"));
+
+// Collect all other arguments, which will be passed to the front end.
+cl::list<std::string> CC1Arguments(cl::ConsumeAfter,
+ cl::desc("<arguments to be passed to front end>..."));
+
+// Option to specify a prefix to be prepended to the header names.
+cl::opt<std::string> HeaderPrefix("prefix", cl::init(""),
+ cl::desc("Prepend header file paths with this prefix."
+ " If not specified,"
+ " the files are considered to be relative to the header list
+file."));
+
int main(int argc, const char **argv) {
- // Figure out command-line arguments.
- if (argc < 2) {
- llvm::errs() << "Usage: modularize <file containing header names> <arguments>\n";
- return 1;
- }
-
+
+ // This causes options to be parsed.
+ cl::ParseCommandLineOptions(argc, argv, "modularize.\n");
+
+ // No go if we have no header list file.
+ if (ListFileName.size() == 0) {
+ cl::PrintHelpMessage();
+ return -1;
+ }
+
+ // By default, use the path component of the list file name.
+ SmallString<256> HeaderDirectory(ListFileName.c_str());
+ sys::path::remove_filename(HeaderDirectory);
+
+ // Get the prefix if we have one.
+ if (HeaderPrefix.size() != 0)
+ HeaderDirectory = HeaderPrefix;
+
// Load the list of headers.
- std::string File = argv[1];
llvm::SmallVector<std::string, 8> Headers;
{
- std::ifstream In(File.c_str());
+ std::ifstream In(ListFileName.c_str());
if (!In) {
- llvm::errs() << "Unable to open header list file \"" << File.c_str() << "\"\n";
+ llvm::errs() << "Unable to open header list file \"" <<
+ ListFileName.c_str() << "\"\n";
return 2;
}
-
std::string Line;
while (std::getline(In, Line)) {
if (Line.empty() || Line[0] == '#')
continue;
-
- Headers.push_back(Line);
+ SmallString<256> headerFileName;
+ if (sys::path::is_absolute(Line))
+ headerFileName = Line;
+ else {
+ headerFileName = HeaderDirectory;
+ sys::path::append(headerFileName, Line);
+ }
+ Headers.push_back(headerFileName.c_str());
}
}
-
+
// Create the compilation database.
+ SmallString<256> PathBuf;
+ llvm::sys::fs::current_path(PathBuf);
llvm::OwningPtr<CompilationDatabase> Compilations;
- {
- std::vector<std::string> Arguments;
- for (int I = 2; I < argc; ++I)
- Arguments.push_back(argv[I]);
- SmallString<256> PathBuf;
- llvm::sys::fs::current_path(PathBuf);
- Compilations.reset(new FixedCompilationDatabase(Twine(PathBuf), Arguments));
- }
-
+ Compilations.reset(new FixedCompilationDatabase(Twine(PathBuf),
+ CC1Arguments));
+
// Parse all of the headers, detecting duplicates.
EntityMap Entities;
ClangTool Tool(*Compilations, Headers);
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list