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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
