[cfe-commits] [PATCH] Add -fixit option to clang-check

Chandler Carruth reviews at llvm-reviews.chandlerc.com
Mon Oct 1 03:31:08 PDT 2012


  Bleh. Way too much boiler plate here. =/


================
Comment at: tools/clang-check/ClangCheck.cpp:93-103
@@ +92,13 @@
+
+class FixItRewriter : public clang::FixItRewriter {
+public:
+  FixItRewriter(clang::DiagnosticsEngine& Diags,
+                clang::SourceManager& SourceMgr,
+                const clang::LangOptions& LangOpts,
+                clang::FixItOptions* FixItOpts)
+      : clang::FixItRewriter(Diags, SourceMgr, LangOpts, FixItOpts) {
+  }
+
+  virtual bool IncludeInDiagnosticCounts() const { return false; }
+};
+
----------------
This class needs a comment about why the existing one won't suffice...

================
Comment at: tools/clang-check/ClangCheck.cpp:108
@@ +107,3 @@
+  virtual bool BeginSourceFileAction(clang::CompilerInstance& CI, StringRef Filename) {
+    FixItOpts.reset(new FixItOptions);
+    Rewriter.reset(new FixItRewriter(CI.getDiagnostics(), CI.getSourceManager(),
----------------
Rather than a whole separate class above, why not just clobber the one option here?

================
Comment at: tools/clang-check/ClangCheck.cpp:65-70
@@ -60,1 +64,8 @@
 
+static cl::opt<bool> Fixit(
+    "fixit",
+    cl::desc(Options->getOptionHelpText(options::OPT_fixit)));
+static cl::opt<bool> FixWhatYouCan(
+    "fix-what-you-can",
+    cl::desc(Options->getOptionHelpText(options::OPT_fix_what_you_can)));
+
----------------
By re-using the existing Clang options, you've made a bit of a surprising change here:

The original 'clang-fixit' tool behaved as if passing both '-fixit' and '-fix-what-you-can' to Clang. The rationale was that because it is a separate explicit step, it is much less problematic to apply partial fixes.

I think that logic continues to hold for a 'clang-check' variant. It seems a bit strange to require the extra flag here. =/ Oh well.

Maybe in the docs somewhere, suggest creating a shell alias for 'clang-fixit' -> 'clang-check -fixit -fix-what-you-can' ?


http://llvm-reviews.chandlerc.com/D51



More information about the cfe-commits mailing list