r211327 - diagtool: refactor TreeView to resemble C++

Jordan Rose jordan_rose at apple.com
Thu Jun 19 18:44:38 PDT 2014


Thanks. Past me hadn't yet grokked good C++, and was still avoiding bad C++.

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




More information about the cfe-commits mailing list