[PATCH] D102728: [clang][Sema] removes -Wfree-nonheap-object reference param false positive

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Oops, looks like I never submitted my replies!



================
Comment at: clang/lib/Sema/SemaChecking.cpp:10466
     if (isa<VarDecl, FunctionDecl>(D))
-      return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr, D);
+      if (!isa<ParmVarDecl>(D) ||
+          !dyn_cast<ParmVarDecl>(D)->getType()->isReferenceType())
----------------
dblaikie wrote:
> It doesn't look like the parameter case is tested, is it? And is it necessary? I think it'd be reasonable to warn on `void f1(int i) { free(&i); }` - so I think it's only the "is it a reference type" that's the important property.
Right, but it's also reasonable to warn on this, which isn't a parameter.
```
int i = 0;
int& r = i;
std::free(&r);
```
However, I will take you up on making sure that parameters are tested.


================
Comment at: clang/test/Sema/warn-free-nonheap-object.cpp:38-40
+  char *ptr1 = (char *)std::malloc(10);
+  char *ptr2 = (char *)std::malloc(10);
+  char *ptr3 = (char *)std::malloc(10);
----------------
dblaikie wrote:
> Is it relevant that these are members? Or could they be globals just as well? Might be simpler/more obvious if they were globals, I think.
This tests that members aren't overlooked, which has a distinct code path to normal variables IIRC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102728