[cfe-commits] [PATCH] Reverted clang-check to fully supported CommandLine Library use-case: global static variables.
Alexander Kornienko
alexfh at google.com
Tue Aug 21 11:47:06 PDT 2012
Even better now: no macros, no include magic and no dependency on linkage
order.
On Fri, Aug 17, 2012 at 5:21 AM, Alexander Kornienko <alexfh at google.com>wrote:
>
> On Fri, Aug 17, 2012 at 11:41 AM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> Er, my suggestion was to make these *static* variables, yes, even though
>> they are in a header file, and then to document thoroughly that this header
>> file is *not* for use within a library, but for use directly within the
>> file defining main.
>
> As Manuel already noted, there was an idea to make definition of these
> variables explicit via a macro. And I didn't find a better way to implement
> that. If you think that a special "include-once-per-binary" header is still
> better, I can switch to this approach.
>
> The current patch is different from that, can you describe how? In general
>> the comments weren't clear enough for me to be confident in how you expect
>> this file to be used...
>>
> Ok, I moved the usage example to the top and added some comments. Is it
> better?
>
>>
>> On Fri, Aug 17, 2012 at 2:26 AM, Manuel Klimek <klimek at google.com> wrote:
>>
>>> +using namespace llvm;
>>>
>>> Remove from header.
>>>
>>> Appart from that, lgtm, minus Chandler's approval of the general idea.
>>>
>>> On Thu, Aug 16, 2012 at 7:17 PM, Alexander Kornienko <alexfh at google.com>
>>> wrote:
>>> > On Thu, Aug 16, 2012 at 7:04 PM, Manuel Klimek <klimek at google.com>
>>> wrote:
>>> >>
>>> >> On Thu, Aug 16, 2012 at 5:45 PM, Alexander Kornienko <
>>> alexfh at google.com>
>>> >> wrote:
>>> >> > Now it's a bit uglier, but doesn't use CommandLine Library in an
>>> >> > unsupported
>>> >> > way.
>>> >> > Chandler, please take a look if it seems better to you.
>>> >>
>>> >> After a short discussion off-list we came to the conclusion that
>>> >> tooling::CommonOptionsParser is a better name for the class, and
>>> >> especially makes the responsibilities clearer...
>>> >
>>> > A new patch is attached.
>>> >
>>> >>
>>> >>
>>> >> > BTW, we still have a number of alien options defined in some llvm
>>> >> > libraries
>>> >> > we link with. Should we try to avoid that by de-globalizing cl
>>> library
>>> >> > or is
>>> >> > someone working on its replacement/refactoring now?
>>> >
>>>
>>
--
Best regards,
Alexander Kornienko
Index: tools/clang/include/clang/Tooling/CommandLineClangTool.h
===================================================================
--- tools/clang/include/clang/Tooling/CommandLineClangTool.h (revision
162110)
+++ tools/clang/include/clang/Tooling/CommandLineClangTool.h (working
copy)
@@ -1,80 +0,0 @@
-//===- CommandLineClangTool.h - command-line clang tools driver -*- C++
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file implements the CommandLineClangTool class used to run clang
-// tools as separate command-line applications with a consistent common
-// interface for handling compilation database and input files.
-//
-// It provides a common subset of command-line options, common algorithm
-// for locating a compilation database and source files, and help messages
-// for the basic command-line interface.
-//
-// It creates a CompilationDatabase, initializes a ClangTool and runs a
-// user-specified FrontendAction over all TUs in which the given files are
-// compiled.
-//
-// This class uses the Clang Tooling infrastructure, see
-// http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
-// for details on setting it up with LLVM source tree.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMANDLINECLANGTOOL_H
-#define LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMANDLINECLANGTOOL_H
-
-#include "llvm/Support/CommandLine.h"
-#include "clang/Tooling/CompilationDatabase.h"
-
-namespace clang {
-
-namespace tooling {
-
-class CompilationDatabase;
-class FrontendActionFactory;
-
-/// \brief A common driver for command-line Clang tools.
-///
-/// Parses a common subset of command-line arguments, locates and loads a
-/// compilation commands database, runs a tool with user-specified action.
It
-/// also contains a help message for the common command-line options.
-/// An example of usage:
-/// @code
-/// int main(int argc, const char **argv) {
-/// CommandLineClangTool Tool;
-/// cl::extrahelp MoreHelp("\nMore help text...");
-/// Tool.initialize(argc, argv);
-/// return Tool.run(newFrontendActionFactory<clang::SyntaxOnlyAction>());
-/// }
-/// @endcode
-///
-class CommandLineClangTool {
-public:
- /// Sets up command-line options and help messages.
- /// Add your own help messages after constructing this tool.
- CommandLineClangTool();
-
- /// Parses command-line, initializes a compilation database.
- /// This method exits program in case of error.
- void initialize(int argc, const char **argv);
-
- /// Runs a clang tool with an action created by \c ActionFactory.
- int run(FrontendActionFactory *ActionFactory);
-
-private:
- llvm::OwningPtr<CompilationDatabase> Compilations;
- llvm::cl::opt<std::string> BuildPath;
- llvm::cl::list<std::string> SourcePaths;
- llvm::cl::extrahelp MoreHelp;
-};
-
-} // namespace tooling
-
-} // namespace clang
-
-#endif // LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMANDLINECLANGTOOL_H
Index: tools/clang/include/clang/Tooling/CommonOptionsParser.h
===================================================================
--- tools/clang/include/clang/Tooling/CommonOptionsParser.h (revision
161861)
+++ tools/clang/include/clang/Tooling/CommonOptionsParser.h (working
copy)
@@ -1,4 +1,4 @@
-//===- CommandLineClangTool.h - command-line clang tools driver -*- C++
-*-===//
+//===- CommonOptionsParser.h - common options for clang tools -*- C++
-*-=====//
//
// The LLVM Compiler Infrastructure
//
@@ -7,74 +7,70 @@
//
//===----------------------------------------------------------------------===//
//
-// This file implements the CommandLineClangTool class used to run clang
-// tools as separate command-line applications with a consistent common
-// interface for handling compilation database and input files.
+// This file implements the CommonOptionsParser class used to parse common
+// command-line options for clang tools, so that they can be run as
separate
+// command-line applications with a consistent common interface for
handling
+// compilation database and input files.
//
// It provides a common subset of command-line options, common algorithm
// for locating a compilation database and source files, and help messages
// for the basic command-line interface.
//
-// It creates a CompilationDatabase, initializes a ClangTool and runs a
-// user-specified FrontendAction over all TUs in which the given files are
-// compiled.
+// It creates a CompilationDatabase and reads common command-line options.
+// An example of usage:
+// @code
+// #include "llvm/Support/CommandLine.h"
+// #include "clang/Tooling/CommonOptionsParser.h"
+// using namespace clang::tooling;
+// using namespace llvm;
//
+// static cl::extrahelp CommonHelp(CommonHelpMessage);
+// static cl::extrahelp MoreHelp("\nMore help text...");
+// static cl:opt<bool> YourOwnOption(...);
+// ...
+//
+// int main(int argc, const char **argv) {
+// CommonOptionsParser OptionsParser(argc, argv);
+// ClangTool Tool(*OptionsParser.Compilations,
OptionsParser.SourcePathList);
+// return Tool.run(newFrontendActionFactory<clang::SyntaxOnlyAction>());
+// }
+// @endcode
+//
// This class uses the Clang Tooling infrastructure, see
// http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
// for details on setting it up with LLVM source tree.
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMANDLINECLANGTOOL_H
-#define LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMANDLINECLANGTOOL_H
+#ifndef LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMONOPTIONSPARSER_H
+#define LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMONOPTIONSPARSER_H
-#include "llvm/Support/CommandLine.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Frontend/FrontendActions.h"
namespace clang {
-
namespace tooling {
-class CompilationDatabase;
-class FrontendActionFactory;
+extern const char *CommonHelpMessage;
-/// \brief A common driver for command-line Clang tools.
+/// \brief A parser for options common to all command-line Clang tools.
///
/// Parses a common subset of command-line arguments, locates and loads a
/// compilation commands database, runs a tool with user-specified action.
It
/// also contains a help message for the common command-line options.
-/// An example of usage:
-/// @code
-/// int main(int argc, const char **argv) {
-/// CommandLineClangTool Tool;
-/// cl::extrahelp MoreHelp("\nMore help text...");
-/// Tool.initialize(argc, argv);
-/// return Tool.run(newFrontendActionFactory<clang::SyntaxOnlyAction>());
-/// }
-/// @endcode
-///
-class CommandLineClangTool {
+class CommonOptionsParser {
public:
- /// Sets up command-line options and help messages.
- /// Add your own help messages after constructing this tool.
- CommandLineClangTool();
+ // Intentionally public.
+ llvm::OwningPtr<CompilationDatabase> Compilations;
+ std::vector<std::string> SourcePathList;
/// Parses command-line, initializes a compilation database.
+ /// This method can change argc and argv contents.
/// This method exits program in case of error.
- void initialize(int argc, const char **argv);
-
- /// Runs a clang tool with an action created by \c ActionFactory.
- int run(FrontendActionFactory *ActionFactory);
-
-private:
- llvm::OwningPtr<CompilationDatabase> Compilations;
- llvm::cl::opt<std::string> BuildPath;
- llvm::cl::list<std::string> SourcePaths;
- llvm::cl::extrahelp MoreHelp;
+ CommonOptionsParser(int &argc, const char **argv);
};
-} // namespace tooling
+} // namespace tooling
+} // namespace clang
-} // namespace clang
-
-#endif // LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMANDLINECLANGTOOL_H
+#endif // LLVM_TOOLS_CLANG_INCLUDE_CLANG_TOOLING_COMMONOPTIONSPARSER_H
Index: tools/clang/tools/clang-check/ClangCheck.cpp
===================================================================
--- tools/clang/tools/clang-check/ClangCheck.cpp (revision 162110)
+++ tools/clang/tools/clang-check/ClangCheck.cpp (working copy)
@@ -1,4 +1,4 @@
-//===- tools/clang-check/ClangCheck.cpp - Clang check tool
----------------===//
+//===--- tools/clang-check/ClangCheck.cpp - Clang check tool
--------------===//
//
// The LLVM Compiler Infrastructure
//
@@ -16,20 +16,19 @@
//
//===----------------------------------------------------------------------===//
-#include "llvm/Support/CommandLine.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/Driver/OptTable.h"
#include "clang/Driver/Options.h"
#include "clang/Frontend/ASTConsumers.h"
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Tooling/CommandLineClangTool.h"
+#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Tooling.h"
using namespace clang::driver;
using namespace clang::tooling;
using namespace llvm;
-static const char *MoreHelpText =
+static cl::extrahelp CommonHelp(CommonHelpMessage);
+static cl::extrahelp MoreHelp(
"\tFor example, to run clang-check on all files in a subtree of the\n"
"\tsource tree, use:\n"
"\n"
@@ -41,26 +40,26 @@
"\n"
"\tNote, that path/in/subtree and current directory should follow
the\n"
"\trules described above.\n"
- "\n";
+ "\n"
+);
+static OwningPtr<OptTable> Options(createDriverOptTable());
+static cl::opt<bool> ASTDump(
+ "ast-dump",
+ cl::desc(Options->getOptionHelpText(options::OPT_ast_dump)));
+static cl::opt<bool> ASTList(
+ "ast-list",
+ cl::desc(Options->getOptionHelpText(options::OPT_ast_list)));
+static cl::opt<bool> ASTPrint(
+ "ast-print",
+ cl::desc(Options->getOptionHelpText(options::OPT_ast_print)));
+static cl::opt<std::string> ASTDumpFilter(
+ "ast-dump-filter",
+ cl::desc(Options->getOptionHelpText(options::OPT_ast_dump_filter)));
+
namespace {
class ActionFactory {
public:
- ActionFactory()
- : Options(createDriverOptTable()),
- ASTDump(
- "ast-dump",
- cl::desc(Options->getOptionHelpText(options::OPT_ast_dump))),
- ASTList(
- "ast-list",
- cl::desc(Options->getOptionHelpText(options::OPT_ast_list))),
- ASTPrint(
- "ast-print",
- cl::desc(Options->getOptionHelpText(options::OPT_ast_print))),
- ASTDumpFilter(
- "ast-dump-filter",
-
cl::desc(Options->getOptionHelpText(options::OPT_ast_dump_filter))) {}
-
clang::ASTConsumer *newASTConsumer() {
if (ASTList)
return clang::CreateASTDeclNodeLister();
@@ -70,19 +69,12 @@
return clang::CreateASTPrinter(&llvm::outs(), ASTDumpFilter);
return new clang::ASTConsumer();
}
-private:
- OwningPtr<OptTable> Options;
- cl::opt<bool> ASTDump;
- cl::opt<bool> ASTList;
- cl::opt<bool> ASTPrint;
- cl::opt<std::string> ASTDumpFilter;
};
}
int main(int argc, const char **argv) {
ActionFactory Factory;
- CommandLineClangTool Tool;
- cl::extrahelp MoreHelp(MoreHelpText);
- Tool.initialize(argc, argv);
+ CommonOptionsParser OptionsParser(argc, argv);
+ ClangTool Tool(*OptionsParser.Compilations,
OptionsParser.SourcePathList);
return Tool.run(newFrontendActionFactory(&Factory));
}
Index: tools/clang/lib/Tooling/CommonOptionsParser.cpp
===================================================================
--- tools/clang/lib/Tooling/CommonOptionsParser.cpp (revision 162110)
+++ tools/clang/lib/Tooling/CommonOptionsParser.cpp (working copy)
@@ -1,4 +1,4 @@
-//===--- CommandLineClangTool.cpp - command-line clang tools driver
-------===//
+//===--- CommonOptionsParser.cpp - common options for clang tools
---------===//
//
// The LLVM Compiler Infrastructure
//
@@ -7,28 +7,31 @@
//
//===----------------------------------------------------------------------===//
//
-// This file implements the CommandLineClangTool class used to run clang
-// tools as separate command-line applications with a consistent common
-// interface for handling compilation database and input files.
+// This file implements the CommonOptionsParser class used to parse common
+// command-line options for clang tools, so that they can be run as
separate
+// command-line applications with a consistent common interface for
handling
+// compilation database and input files.
//
// It provides a common subset of command-line options, common algorithm
// for locating a compilation database and source files, and help messages
// for the basic command-line interface.
//
-// It creates a CompilationDatabase, initializes a ClangTool and runs a
-// user-specified FrontendAction over all TUs in which the given files are
-// compiled.
+// It creates a CompilationDatabase and reads common command-line options.
//
+// This class uses the Clang Tooling infrastructure, see
+// http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+// for details on setting it up with LLVM source tree.
+//
//===----------------------------------------------------------------------===//
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Tooling/CommandLineClangTool.h"
+#include "llvm/Support/CommandLine.h"
+#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Tooling.h"
using namespace clang::tooling;
using namespace llvm;
-static const char *MoreHelpText =
+const char *clang::tooling::CommonHelpMessage =
"\n"
"-p <build-path> is used to read a compile command database.\n"
"\n"
@@ -40,26 +43,27 @@
"\thttp://clang.llvm.org/docs/HowToSetupToolingForLLVM.html for an\n"
"\texample of setting up Clang Tooling on a source tree.\n"
"\n"
- "<source0> ... specify the paths of source files. These paths are
looked\n"
- "\tup in the compile command database. If the path of a file is
absolute,\n"
- "\tit needs to point into CMake's source tree. If the path is
relative,\n"
- "\tthe current working directory needs to be in the CMake source tree
and\n"
- "\tthe file must be in a subdirectory of the current working
directory.\n"
- "\t\"./\" prefixes in the relative files will be automatically
removed,\n"
- "\tbut the rest of a relative path must be a suffix of a path in the\n"
- "\tcompile command database.\n"
+ "<source0> ... specify the paths of source files. These paths are\n"
+ "\tlooked up in the compile command database. If the path of a file
is\n"
+ "\tabsolute, it needs to point into CMake's source tree. If the path
is\n"
+ "\trelative, the current working directory needs to be in the CMake\n"
+ "\tsource tree and the file must be in a subdirectory of the current\n"
+ "\tworking directory. \"./\" prefixes in the relative files will be\n"
+ "\tautomatically removed, but the rest of a relative path must be a\n"
+ "\tsuffix of a path in the compile command database.\n"
"\n";
-CommandLineClangTool::CommandLineClangTool() :
- BuildPath("p", cl::desc("Build path"), cl::Optional),
- SourcePaths(cl::Positional, cl::desc("<source0> [... <sourceN>]"),
- cl::OneOrMore),
- MoreHelp(MoreHelpText) {
-}
+static cl::opt<std::string> BuildPath(
+ "p", cl::desc("Build path"), cl::Optional);
-void CommandLineClangTool::initialize(int argc, const char **argv) {
- Compilations.reset(FixedCompilationDatabase::loadFromCommandLine(argc,
argv));
+static cl::list<std::string> SourcePaths(
+ cl::Positional, cl::desc("<source0> [... <sourceN>]"), cl::OneOrMore);
+
+CommonOptionsParser::CommonOptionsParser(int &argc, const char **argv) {
+ Compilations.reset(FixedCompilationDatabase::loadFromCommandLine(argc,
+ argv));
cl::ParseCommandLineOptions(argc, argv);
+ SourcePathList = SourcePaths;
if (!Compilations) {
std::string ErrorMessage;
if (!BuildPath.empty()) {
@@ -73,8 +77,3 @@
llvm::report_fatal_error(ErrorMessage);
}
}
-
-int CommandLineClangTool::run(FrontendActionFactory *ActionFactory) {
- ClangTool Tool(*Compilations, SourcePaths);
- return Tool.run(ActionFactory);
-}
Index: tools/clang/lib/Tooling/CommandLineClangTool.cpp
===================================================================
--- tools/clang/lib/Tooling/CommandLineClangTool.cpp (revision 162110)
+++ tools/clang/lib/Tooling/CommandLineClangTool.cpp (working copy)
@@ -1,80 +0,0 @@
-//===--- CommandLineClangTool.cpp - command-line clang tools driver
-------===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file implements the CommandLineClangTool class used to run clang
-// tools as separate command-line applications with a consistent common
-// interface for handling compilation database and input files.
-//
-// It provides a common subset of command-line options, common algorithm
-// for locating a compilation database and source files, and help messages
-// for the basic command-line interface.
-//
-// It creates a CompilationDatabase, initializes a ClangTool and runs a
-// user-specified FrontendAction over all TUs in which the given files are
-// compiled.
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Tooling/CommandLineClangTool.h"
-#include "clang/Tooling/Tooling.h"
-
-using namespace clang::tooling;
-using namespace llvm;
-
-static const char *MoreHelpText =
- "\n"
- "-p <build-path> is used to read a compile command database.\n"
- "\n"
- "\tFor example, it can be a CMake build directory in which a file
named\n"
- "\tcompile_commands.json exists (use
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON\n"
- "\tCMake option to get this output). When no build path is
specified,\n"
- "\tclang-check will attempt to locate it automatically using all
parent\n"
- "\tpaths of the first input file. See:\n"
- "\thttp://clang.llvm.org/docs/HowToSetupToolingForLLVM.html for an\n"
- "\texample of setting up Clang Tooling on a source tree.\n"
- "\n"
- "<source0> ... specify the paths of source files. These paths are
looked\n"
- "\tup in the compile command database. If the path of a file is
absolute,\n"
- "\tit needs to point into CMake's source tree. If the path is
relative,\n"
- "\tthe current working directory needs to be in the CMake source tree
and\n"
- "\tthe file must be in a subdirectory of the current working
directory.\n"
- "\t\"./\" prefixes in the relative files will be automatically
removed,\n"
- "\tbut the rest of a relative path must be a suffix of a path in the\n"
- "\tcompile command database.\n"
- "\n";
-
-CommandLineClangTool::CommandLineClangTool() :
- BuildPath("p", cl::desc("Build path"), cl::Optional),
- SourcePaths(cl::Positional, cl::desc("<source0> [... <sourceN>]"),
- cl::OneOrMore),
- MoreHelp(MoreHelpText) {
-}
-
-void CommandLineClangTool::initialize(int argc, const char **argv) {
- Compilations.reset(FixedCompilationDatabase::loadFromCommandLine(argc,
argv));
- cl::ParseCommandLineOptions(argc, argv);
- if (!Compilations) {
- std::string ErrorMessage;
- if (!BuildPath.empty()) {
- Compilations.reset(CompilationDatabase::autoDetectFromDirectory(
- BuildPath, ErrorMessage));
- } else {
- Compilations.reset(CompilationDatabase::autoDetectFromSource(
- SourcePaths[0], ErrorMessage));
- }
- if (!Compilations)
- llvm::report_fatal_error(ErrorMessage);
- }
-}
-
-int CommandLineClangTool::run(FrontendActionFactory *ActionFactory) {
- ClangTool Tool(*Compilations, SourcePaths);
- return Tool.run(ActionFactory);
-}
Index: tools/clang/lib/Tooling/CMakeLists.txt
===================================================================
--- tools/clang/lib/Tooling/CMakeLists.txt (revision 162110)
+++ tools/clang/lib/Tooling/CMakeLists.txt (working copy)
@@ -2,7 +2,7 @@
add_clang_library(clangTooling
ArgumentsAdjusters.cpp
- CommandLineClangTool.cpp
+ CommonOptionsParser.cpp
CompilationDatabase.cpp
Refactoring.cpp
RefactoringCallbacks.cpp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120821/743f2d79/attachment.html>
More information about the cfe-commits
mailing list