Szelethus added inline comments.

================
Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:146-149
+/// If you'd like to add a new -cc1 flag, add it to
+/// include/clang/Driver/CC1Options.td, add a new field to store the value of
+/// that flag in this class, and initialize it in
+/// lib/Frontend/CompilerInvocation.cpp.
----------------
NoQ wrote:
> Nono, don't add more -cc1 flags :)
Code review is there to stop adding unnecessary -cc1 flags. Are we sure we 
wouldn't even like to document it? I myself will add at least 2 more -cc1 flags 
in the future (-analyzer-config-help, -analyzer-checker-option-help), that 
undoubtedly belong there.


================
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:172
+                     .getAsInteger(10, *V);
+  assert(!HasFailed && "analyzer-config option should be numeric");
+  (void)HasFailed;
----------------
NoQ wrote:
> I guess @xazax.hun's comment also applies to this assert.
Which is sadly only going to be addressed in followup patches :/ Note that I 
merely moved code around in this patch, but have organized things in a way that 
emitting proper diagnostics will be super easy going forward. Since analyzer 
configs are mainly meant for developers, I think this is acceptable before I 
fix it (in any case, it has always been like this).


https://reviews.llvm.org/D53483



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to