NoQ added a comment. In https://reviews.llvm.org/D20811#544250, @dcoughlin wrote:
> I think a good rule of thumb for readability is: suppose you are a maintainer > and need to add a summary for a new function. Can you copy the the summary > for an existing function and figure out what each component means so you can > change it for the new function? Seems i've written too many summaries to reliably use this rule :) Could you have a look at another attempt?: SUMMARY(isalnum, ARGUMENT_TYPES { IntTy }, RETURN_TYPE(IntTy), INVALIDATION_APPROACH(EvalCallAsPure)) CASE // Boils down to isupper() or islower() or isdigit() PRE_CONDITION(ARG_NO(0), CONDITION_KIND(WithinRange)) RANGE('0', '9') RANGE('A', 'Z') RANGE('a', 'z') END_PRE_CONDITION POST_CONDITION(OutOfRange) VALUE(0) END_POST_CONDITION END_CASE CASE // The locale-specific range. PRE_CONDITION(ARG_NO(0), CONDITION_KIND(WithinRange)) RANGE(128, 255) END_PRE_CONDITION // No post-condition. We are completely unaware of // locale-specific return values. END_CASE CASE PRE_CONDITION(ARG_NO(0), CONDITION_KIND(OutOfRange)) RANGE('0', '9') RANGE('A', 'Z') RANGE('a', 'z') RANGE(128, 255) END_PRE_CONDITION POST_CONDITION(WithinRange) VALUE(0) END_POST_CONDITION END_CASE END_SUMMARY ================ Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:419 @@ -418,1 +418,3 @@ +def StdLibraryFunctionsChecker : Checker<"StdLibraryFunctions">, + HelpText<"Improve modeling of standard library functions">, ---------------- dcoughlin wrote: > I know you and Gábor already discussed this -- but shouldn't this be > CStdLibraryFunctionsChecker or 'StdCLibraryFunctionsChecker'? Or is is your > intent that both C and C++ standard libraries would be modeled by this > checker? Hmm, i just realized what you guys were talking about :) The same checker cpp file and even the same checker object should probably produce different checker list entries here which would go into separate packages (cplusplus for C++ library functions, etc.). We could even split the specifications into different files, but the checker object would still be the same, defined in the same file. Will do. ================ Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:537 @@ +536,3 @@ + SPEC_DATA { + ARGUMENT_TYPES { IntTy }, + RETURN_TYPE(IntTy), ---------------- dcoughlin wrote: > The argument and return types seem like more a property of the function than > than the summary. Why are they here and not with the function name? Because this is where C++ initializer list syntax forces them to be. Hiding this detail is, as far as i see, only possible with the means of BEGIN_.../END_... macros (which isn't a big deal i think). ================ Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:547 @@ +546,3 @@ + RANGE { + RET_VAL, RANGE_KIND(OutOfRange), + SET { POINT(0) } ---------------- dcoughlin wrote: > Is it ever the case that this final 'RANGE" constrains anything other than > the return value? If not, can 'RET_VAL' be elided? Some summaries only have pre-conditions: "for this argument constraint, any return value is possible". We should also be able to support void functions, which have no return values. https://reviews.llvm.org/D20811 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits