[PATCH] D94640: adds more checks to -Wfree-nonheap-object

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Sorry for the late review.



================
Comment at: clang/lib/Sema/SemaChecking.cpp:10294-10303
+void CheckFreeArgumentsPlus(Sema &S, const std::string &CalleeName,
+                            const UnaryOperator *UnaryExpr) {
+  const auto *Lambda = dyn_cast<LambdaExpr>(
+      UnaryExpr->getSubExpr()->IgnoreImplicitAsWritten()->IgnoreParens());
+  if (!Lambda)
     return;
 
----------------
This special case is rather too special, and I don't think it's worth the lines of code we're spending on it.

There's a fair amount of work being done here to figure out what's being passed to `free`, and some of it, like this, seems pretty hard to justify in isolation. I wonder if we can reuse some of our existing mechanisms -- for example, can we run the constant evaluator on the operand to `free` instead, and see if it evaluates to a known non-heap object?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:10326-10328
+    OS << '\'';
+    Cast->printPretty(OS, nullptr, S.getPrintingPolicy());
+    OS << '\'';
----------------
Please don't pretty-print source expressions and include them in the diagnostic text. Instead, include a source range to highlight them in the caret diagnostic. See https://clang.llvm.org/docs/InternalsManual.html#the-format-string for our diagnostic best practices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94640