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

Reply via email to