r211327 - diagtool: refactor TreeView to resemble C++
Alp Toker
alp at nuanti.com
Sat Jun 21 09:29:31 PDT 2014
On 20/06/2014 04:44, Jordan Rose wrote:
> Thanks. Past me hadn't yet grokked good C++, and was still avoiding bad C++.
What matters is that past Jordan grokked the right design patterns,
making light work of this tidy-up when it finally came.
+1 for eventual consistency :-)
Alp.
>
> Jordan
>
> On Jun 19, 2014, at 17:06 , Alp Toker <alp at nuanti.com> wrote:
>
>> Author: alp
>> Date: Thu Jun 19 19:06:42 2014
>> New Revision: 211327
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=211327&view=rev
>> Log:
>> diagtool: refactor TreeView to resemble C++
>>
>> Replace lots of old-school parameter passing with neat class members.
>> No attempt made yet to modernize loops, but it's a start.
>>
>> Modified:
>> cfe/trunk/tools/diagtool/TreeView.cpp
>>
>> Modified: cfe/trunk/tools/diagtool/TreeView.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/diagtool/TreeView.cpp?rev=211327&r1=211326&r2=211327&view=diff
>> ==============================================================================
>> --- cfe/trunk/tools/diagtool/TreeView.cpp (original)
>> +++ cfe/trunk/tools/diagtool/TreeView.cpp Thu Jun 19 19:06:42 2014
>> @@ -6,10 +6,6 @@
>> // License. See LICENSE.TXT for details.
>> //
>> //===----------------------------------------------------------------------===//
>> -//
>> -// This diagnostic tool
>> -//
>> -//===----------------------------------------------------------------------===//
>>
>> #include "DiagTool.h"
>> #include "DiagnosticNames.h"
>> @@ -22,118 +18,129 @@
>> #include "llvm/Support/Format.h"
>> #include "llvm/Support/Process.h"
>>
>> -DEF_DIAGTOOL("tree",
>> - "Show warning flags in a tree view",
>> - TreeView)
>> -
>> +DEF_DIAGTOOL("tree", "Show warning flags in a tree view", TreeView)
>> +
>> using namespace clang;
>> using namespace diagtool;
>>
>> -static void printUsage() {
>> - llvm::errs() << "Usage: diagtool tree [--flags-only] [<diagnostic-group>]\n";
>> -}
>> -
>> -static bool showColors(llvm::raw_ostream &out) {
>> +static bool hasColors(const llvm::raw_ostream &out) {
>> if (&out != &llvm::errs() && &out != &llvm::outs())
>> return false;
>> return llvm::errs().is_displayed() && llvm::outs().is_displayed();
>> }
>>
>> -static void setColor(bool ShowColors, llvm::raw_ostream &out,
>> - llvm::raw_ostream::Colors Color) {
>> - if (ShowColors)
>> - out << llvm::sys::Process::OutputColor(Color, false, false);
>> -}
>> +class TreePrinter {
>> +public:
>> + llvm::raw_ostream &out;
>> + const bool ShowColors;
>> + bool FlagsOnly;
>>
>> -static void resetColor(bool ShowColors, llvm::raw_ostream &out) {
>> - if (ShowColors)
>> - out << llvm::sys::Process::ResetColor();
>> -}
>> + TreePrinter(llvm::raw_ostream &out)
>> + : out(out), ShowColors(hasColors(out)), FlagsOnly(false) {}
>>
>> -static bool isIgnored(unsigned DiagID) {
>> - // FIXME: This feels like a hack.
>> - static clang::DiagnosticsEngine Diags(new DiagnosticIDs,
>> - new DiagnosticOptions);
>> - return Diags.isIgnored(DiagID, SourceLocation());
>> -}
>> + void setColor(llvm::raw_ostream::Colors Color) {
>> + if (ShowColors)
>> + out << llvm::sys::Process::OutputColor(Color, false, false);
>> + }
>> +
>> + void resetColor() {
>> + if (ShowColors)
>> + out << llvm::sys::Process::ResetColor();
>> + }
>> +
>> + static bool isIgnored(unsigned DiagID) {
>> + // FIXME: This feels like a hack.
>> + static clang::DiagnosticsEngine Diags(new DiagnosticIDs,
>> + new DiagnosticOptions);
>> + return Diags.isIgnored(DiagID, SourceLocation());
>> + }
>>
>> -static void printGroup(llvm::raw_ostream &out, const GroupRecord &Group,
>> - bool FlagsOnly, unsigned Indent = 0) {
>> - out.indent(Indent * 2);
>> -
>> - bool ShowColors = showColors(out);
>> - setColor(ShowColors, out, llvm::raw_ostream::YELLOW);
>> - out << "-W" << Group.getName() << "\n";
>> - resetColor(ShowColors, out);
>> -
>> - ++Indent;
>> - for (GroupRecord::subgroup_iterator I = Group.subgroup_begin(),
>> - E = Group.subgroup_end();
>> - I != E; ++I) {
>> - printGroup(out, *I, FlagsOnly, Indent);
>> - }
>> -
>> - if (!FlagsOnly) {
>> - for (GroupRecord::diagnostics_iterator I = Group.diagnostics_begin(),
>> - E = Group.diagnostics_end();
>> + void printGroup(const GroupRecord &Group, unsigned Indent = 0) {
>> + out.indent(Indent * 2);
>> +
>> + setColor(llvm::raw_ostream::YELLOW);
>> + out << "-W" << Group.getName() << "\n";
>> + resetColor();
>> +
>> + ++Indent;
>> + for (GroupRecord::subgroup_iterator I = Group.subgroup_begin(),
>> + E = Group.subgroup_end();
>> I != E; ++I) {
>> - if (ShowColors) {
>> - if (!isIgnored(I->DiagID)) {
>> - setColor(ShowColors, out, llvm::raw_ostream::GREEN);
>> - }
>> + printGroup(*I, Indent);
>> + }
>> +
>> + if (!FlagsOnly) {
>> + for (GroupRecord::diagnostics_iterator I = Group.diagnostics_begin(),
>> + E = Group.diagnostics_end();
>> + I != E; ++I) {
>> + if (ShowColors && !isIgnored(I->DiagID))
>> + setColor(llvm::raw_ostream::GREEN);
>> + out.indent(Indent * 2);
>> + out << I->getName();
>> + resetColor();
>> + out << "\n";
>> }
>> - out.indent(Indent * 2);
>> - out << I->getName();
>> - resetColor(ShowColors, out);
>> - out << "\n";
>> }
>> }
>> -}
>>
>> -static int showGroup(llvm::raw_ostream &out, StringRef RootGroup,
>> - bool FlagsOnly) {
>> - ArrayRef<GroupRecord> AllGroups = getDiagnosticGroups();
>> -
>> - if (RootGroup.size() > UINT16_MAX) {
>> - llvm::errs() << "No such diagnostic group exists\n";
>> - return 1;
>> - }
>> -
>> - const GroupRecord *Found =
>> - std::lower_bound(AllGroups.begin(), AllGroups.end(), RootGroup);
>> -
>> - if (Found == AllGroups.end() || Found->getName() != RootGroup) {
>> - llvm::errs() << "No such diagnostic group exists\n";
>> - return 1;
>> - }
>> -
>> - printGroup(out, *Found, FlagsOnly);
>> -
>> - return 0;
>> -}
>> + int showGroup(StringRef RootGroup) {
>> + ArrayRef<GroupRecord> AllGroups = getDiagnosticGroups();
>> +
>> + if (RootGroup.size() > UINT16_MAX) {
>> + llvm::errs() << "No such diagnostic group exists\n";
>> + return 1;
>> + }
>>
>> -static int showAll(llvm::raw_ostream &out, bool FlagsOnly) {
>> - ArrayRef<GroupRecord> AllGroups = getDiagnosticGroups();
>> - llvm::DenseSet<unsigned> NonRootGroupIDs;
>> + const GroupRecord *Found =
>> + std::lower_bound(AllGroups.begin(), AllGroups.end(), RootGroup);
>>
>> - for (ArrayRef<GroupRecord>::iterator I = AllGroups.begin(),
>> - E = AllGroups.end();
>> - I != E; ++I) {
>> - for (GroupRecord::subgroup_iterator SI = I->subgroup_begin(),
>> - SE = I->subgroup_end();
>> - SI != SE; ++SI) {
>> - NonRootGroupIDs.insert((unsigned)SI.getID());
>> + if (Found == AllGroups.end() || Found->getName() != RootGroup) {
>> + llvm::errs() << "No such diagnostic group exists\n";
>> + return 1;
>> }
>> +
>> + printGroup(*Found);
>> +
>> + return 0;
>> }
>>
>> - assert(NonRootGroupIDs.size() < AllGroups.size());
>> + int showAll() {
>> + ArrayRef<GroupRecord> AllGroups = getDiagnosticGroups();
>> + llvm::DenseSet<unsigned> NonRootGroupIDs;
>> +
>> + for (ArrayRef<GroupRecord>::iterator I = AllGroups.begin(),
>> + E = AllGroups.end();
>> + I != E; ++I) {
>> + for (GroupRecord::subgroup_iterator SI = I->subgroup_begin(),
>> + SE = I->subgroup_end();
>> + SI != SE; ++SI) {
>> + NonRootGroupIDs.insert((unsigned)SI.getID());
>> + }
>> + }
>> +
>> + assert(NonRootGroupIDs.size() < AllGroups.size());
>>
>> - for (unsigned i = 0, e = AllGroups.size(); i != e; ++i) {
>> - if (!NonRootGroupIDs.count(i))
>> - printGroup(out, AllGroups[i], FlagsOnly);
>> + for (unsigned i = 0, e = AllGroups.size(); i != e; ++i) {
>> + if (!NonRootGroupIDs.count(i))
>> + printGroup(AllGroups[i]);
>> + }
>> +
>> + return 0;
>> + }
>> +
>> + void showKey() {
>> + if (ShowColors) {
>> + out << '\n';
>> + setColor(llvm::raw_ostream::GREEN);
>> + out << "GREEN";
>> + resetColor();
>> + out << " = enabled by default\n\n";
>> + }
>> }
>> +};
>>
>> - return 0;
>> +static void printUsage() {
>> + llvm::errs() << "Usage: diagtool tree [--flags-only] [<diagnostic-group>]\n";
>> }
>>
>> int TreeView::run(unsigned int argc, char **argv, llvm::raw_ostream &out) {
>> @@ -147,7 +154,7 @@ int TreeView::run(unsigned int argc, cha
>> ++argv;
>> }
>> }
>> -
>> +
>> bool ShowAll = false;
>> StringRef RootGroup;
>>
>> @@ -168,17 +175,8 @@ int TreeView::run(unsigned int argc, cha
>> return -1;
>> }
>>
>> - if (showColors(out)) {
>> - out << '\n';
>> - setColor(true, out, llvm::raw_ostream::GREEN);
>> - out << "GREEN";
>> - resetColor(true, out);
>> - out << " = enabled by default\n\n";
>> - }
>> -
>> - if (ShowAll)
>> - return showAll(out, FlagsOnly);
>> -
>> - return showGroup(out, RootGroup, FlagsOnly);
>> + TreePrinter TP(out);
>> + TP.FlagsOnly = FlagsOnly;
>> + TP.showKey();
>> + return ShowAll ? TP.showAll() : TP.showGroup(RootGroup);
>> }
>> -
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
More information about the cfe-commits
mailing list