================
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(),
----------------
Chandler Carruth wrote:
> Rather than a whole separate class above, why not just clobber the one option
> here?
Because all implementations (preferable we would reuse FixItRewriteInPlace) are
defined in an unnamed namespace inside
lib/Rewrite/Frontend/FrontendActions.cpp. Would you rather, I move those into
clang/Rewrite/Frontend/FrontendActions.h?
Also, we cannot just call clang::FixItAction::BeginSourceFileAction() as it
installs clang::FixItRewriter() which we don't want.
================
Comment at: tools/clang-check/ClangCheck.cpp:107
@@ +106,3 @@
+public:
+ virtual bool BeginSourceFileAction(clang::CompilerInstance& CI, StringRef
Filename) {
+ FixItOpts.reset(new FixItOptions);
----------------
Manuel Klimek wrote:
> 80 columns.
Done.
================
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; }
+};
+
----------------
Chandler Carruth wrote:
> This class needs a comment about why the existing one won't suffice...
Done.
================
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)));
+
----------------
Chandler Carruth wrote:
> 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' ?
I agree that it is not ideal, but I would vote for being as consistent with the
bare clang as possible. I will update the docs once this is submitted.
================
Comment at: tools/clang-check/ClangCheck.cpp:135
@@ -78,3 +134,3 @@
int main(int argc, const char **argv) {
clang_check::ClangCheckActionFactory Factory;
CommonOptionsParser OptionsParser(argc, argv);
----------------
Manuel Klimek wrote:
> I'd change the scope to after if (Fixit).
Done.
http://llvm-reviews.chandlerc.com/D51
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
