NagyDonat wrote: > Based on your review of the code do you have any suggestions on how I should > go about tightening the heuristic?
Your code puts pointers (that were allocated by `new` or `new[]`) into a relinquished state when they are passed to a constructor call. Currently this is overly broad, because it activates for _all_ constructor calls and the implementation cannot distinguish classes that take ownership and properly release the resources in their destructor (e.g. `unique_ptr` or the class `Bar` defined in your tests) and classes that don't do this (e.g. `BarNoDelete` in your tests). To fix the concrete bug that was reported in https://github.com/llvm/llvm-project/issues/153300 it is sufficient to recognize (by checking the class name) that `unique_ptr` properly releases the pointee -- which is already implemented in https://github.com/llvm/llvm-project/pull/152751. That commit seems to be the most practical variant of this "relinquish pointers passed to a constructor" approach, so unfortunately my primary suggestion is that this pull request should be abandoned. I understand that it is annoying when some work turns out to be redundant; but unfortunately this is a part of decentralized development. (In fact, a few days ago I was in the same shoes when my commit https://github.com/llvm/llvm-project/pull/155237 had to be abandoned because somebody else fixed that bug a bit earlier than me.) ---- Of course recognizing the name of `unique_ptr` is just quick workaround for fixing https://github.com/llvm/llvm-project/issues/153300 -- the more elegant solution would be ensuring that the analyzer properly simulates the execution of the destructor `~unique_ptr` and sees that the memory is released within that destructor. (In theory the analyzer should simulate the execution of destructors, but the code that is responsible for this is fragile and probably fails in many corner cases.) In this direction you could follow the observations in https://github.com/llvm/llvm-project/issues/153300#issuecomment-3190160492 to develop a "proper solution" that fixes the underlying issue which prevents the simulated execution of the destructor. This may be a difficult approach (you'd need to tweak tricky internals of the analyzer engine), but if you manage to develop a solution, then that would be an useful improvement of the analyzer. https://github.com/llvm/llvm-project/pull/155131 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
