xazax.hun added a comment. Overall looks good if the community agrees with the directions. Some comments inline.
================ Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:243 + /// specified. + StringRef getStringOption(StringRef Name, StringRef DefaultVal); ---------------- If you want the devs to maintain an explicit getter for each analyzer option rather than making this one public at some point, please document expectation this somewhere. ================ Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:157 + .getAsInteger(10, Ret); + assert(!HasFailed && "analyzer-config option should be numeric"); + (void)HasFailed; ---------------- Can this assert be triggered using a bad invocation of the analyzer? I wonder if it is a good idea to use asserts to validate user input. Maybe it would be better to generate a warning and return the default value? ================ Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:205 + .getAsInteger(10, Ret); + assert(!HasFailed && "analyzer-config option should be numeric"); + (void)HasFailed; ---------------- Same as above. Repository: rC Clang https://reviews.llvm.org/D53483 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
