[PATCH] Fix memory leak for APValues that do memory allocation.

[Manuel Klimek]

================
Comment at: lib/AST/APValue.cpp:235-240
@@ +234,8 @@
+    return getComplexFloatReal().needsCleanup();
+  case ComplexInt:
+    assert(!getComplexIntReal().needsCleanup() &&
+           "_Complex cannot be created with large enough real values.");
+    assert(!getComplexIntImag().needsCleanup() &&
+           "_Complex cannot be created with large enough real values.");
+    return false;
+  case LValue:
----------------
Richard Smith wrote:
> Manuel Klimek wrote:
> > Richard Smith wrote:
> > > This seems likely to bite us in the future. Please implement this "properly" rather than asserting, even though it can't happen today.
> > So, do we need to check both here? Or is only one enough? Given how little I understand about the code I'm wary of implementing anything without being able to write any tests.
> > I'll give in eventually, but my gut feeling tells me that I'd want to know in the future when this changes, so I can go and write some test for it...
> Just checking one of them is fine. How about adding a test to your existing test suite which just checks that compile fails for _Complex __int128, with a comment indicating that it shouldn't leak if it works? That way, whoever adds support for that will know to update the test.
Implemented like you proposed. I still expect this to bite us in the future more than not implementing it (for example, from what I understand of the code I'd expect I can get one where real needsCleanup() but imag doesn't, but I'm not sure).


http://llvm-reviews.chandlerc.com/D736

BRANCH
  memory-leak

ARCANIST PROJECT
  clang