Szelethus accepted this revision. Szelethus added subscribers: gribozavr, aaron.ballman, alexfh. Szelethus added a comment.
Hmm, why the need for checker options? Why not have them by default? If fixits are an experimental feature, maybe we should have a global `enable-fixits` config maybe. But I don't insist :) Now that we proposed that clang-tidy should also use the BugReporter, maybe we should let some clang-tidy folks chip in on this. In any case, a direction is set and looks great. ================ Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:879 + ArrayRef<FixItHint> getFixits() const { return Fixits; } + ---------------- Hmm, will this return an immutable container? If not, can we make it so? ================ Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:743 + assert(!Hint.isNull()); + // FIXME: Add support for InsertFromRange and BeforePreviousInsertion. + o << " <dict>\n"; ---------------- Shouldn't we assert this? ================ Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:84 namespace { class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer { DiagnosticsEngine &Diag; ---------------- Oh my god. This must be a runner up for "Worst class name in the Clang Static Analyzer", and might even all the trophies home. I wanted to make a joke about honorable mentions, but nothing compares :^) ================ Comment at: clang/test/Analysis/objc-arc.m:1 -// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -o %t.plist %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist %s // RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/objc-arc.m.plist - ---------------- Just thinking aloud, but maybe it'd be time to create a `RUN:` line formatter for test files, WDYT? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits