Szelethus added a comment.

The idea is noble with the addition of `validate` functions, assert in debug 
builds and just move on in release. However, I'd expect it to be integrated 
into the signature matching function.



================
Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:234-236
+  // The signature of a function we want to describe with a summary. This is a
+  // concessive signature, meaning there may be irrelevant types in the
+  // signature which we do not check against a function with concrete types.
----------------
It might be worth putting a `TODO` here to not forget the constness methods :^)


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:697
         if (auto *FD = dyn_cast<FunctionDecl>(D)) {
-          if (S.matchesSignature(FD)) {
+          if (S.Sign.matches(FD) && S.validate(FD)) {
             auto Res = Map.insert({FD->getCanonicalDecl(), S});
----------------
This looks a bit odd, we're checking whether the function matches, and than we 
validate right after? Shouldn't we just not match the `FD` if it isn't valid?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77658



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

Reply via email to