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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Thanks for following up on this.

- Probably best to commit the revert of the workarounds as a separate follow-up commit, but I appreciate seeing them here to understand that this patch addresses them
-



================
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())
----------------
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.


================
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);
----------------
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.


================
Comment at: clang/test/Sema/warn-free-nonheap-object.cpp:43-46
+  static void free_reference(char &x) { ::free(&x); }
+  static void free_reference(char &&x) { ::free(&x); }
+  static void std_free_reference(char &x) { std::free(&x); }
+  static void std_free_reference(char &&x) { std::free(&x); }
----------------
Are these notably different from the non-member examples above? I assume not & so I'd probably skip these test cases.


================
Comment at: clang/test/Sema/warn-free-nonheap-object.cpp:69-70
+  {
+    char* P = (char *)std::malloc(2);
+    free_reference(*P);
+  }
----------------
I don't think this code is necessary - since clang warnings aren't interprocedural, there's no need to flesh out the example this far - you can have the function standalone elsewhere, that takes a reference parameter and tries to free it after taking its address. Without any caller.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102728