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
