[PATCH] D62556: [analyzer] NFC: CallDescription: Implement describing C library functions.

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 5 01:46:38 PDT 2019


xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

Once Aleksei's comments are resolved it is good to go. My comments are notes and not requests.



================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1052
+  /// number of arguments.
+  CDF_MaybeBuiltin = 1 << 0,
+};
----------------
I wonder if this is the right name. Do checker authors know the relationship between a standard library function and builtins? Maybe something like CDF_IsCStandardLibFunc would be more prescriptive when to set this? Or do we have non standard library builtins that we might want to match using this facility? The comment above is great, I just wonder if checker authors will read that. In case it does show up in the doxygen feel free to leave the name as is.



================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:368
+  if (CD.Flags & CDF_MaybeBuiltin) {
+    return CheckerContext::isCLibraryFunction(FD, CD.getFunctionName()) &&
+           (CD.RequiredArgs == CallDescription::NoArgRequirement ||
----------------
In case this happens to be a performance problem we could cache the builtin id in the future. This is just a note, do not need to optimize this prematurely.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62556/new/

https://reviews.llvm.org/D62556





More information about the cfe-commits mailing list