<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
I'm afraid this change is going to open a big can of worms. You are
essentially promote <br>
private member function to be public. Such practice should be
generally avoided even for <br>
the local/private class which is used in small scope, let alone
these fundamental structures <br>
which are extensively used in the compiler. <br>
<br>
> object needs the destructor called after
its memory was freed
<br>
<br>
If the block of memory containing APFloat/APInt is already freed,
how do you know <br>
the state of these objects are still valid? Depending on the
implementation, free-operation <br>
might clobber the memory with some special value for debugging
purpose. If it were safe<br>
to call needsCleanup(), why not directly call the destructor? Is
needsCleanup() sufficient <br>
and/or necessary condition for calling the destructor? How do we
keep them in sync in the <br>
future?<br>
<br>
I think you have to restructure the code that using APFloat/APInt to
avoid memory leak. <br>
I certainly understand that restructuring sometimes is not easy
undertaking, however, <br>
we have no other way around.<br>
<br>
BTW, by following the link, I take a glimpse of APValue, and spot a
potential problem there <br>
(I think it is irrelevant to the problem you are trying to tackle).
<br>
<br>
The union member "Data" will be cost to APFloat/APInt etc, and the
"Aligner" is the dummy <br>
field used to make sure the union sitting at right place. However,
how do we know <br>
the alignof(APFloat|APInt|etc) is no more stringent than
alignof(void*)?<br>
Shouldn't we use AlignedCharArrayUnion for this purpose?<br>
<br>
cat -n tools/clang/include/clang/AST/APValue.h<br>
38 class APValue {<br>
...<br>
<br>
116 union {<br>
117 void *Aligner;<br>
118 char Data[MaxSize];<br>
119 };<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 5/12/13 10:59 PM, Manuel Klimek
wrote:<br>
</div>
<blockquote
cite="mid:differential-rev-PHID-DREV-hbqxhqypgrmnrcqeu5te-req@llvm-reviews.chandlerc.com"
type="cite">
<pre wrap="">This is needed so one can check if the object needs the destructor called after
its memory was freed. See <a class="moz-txt-link-freetext" href="http://llvm-reviews.chandlerc.com/D736">http://llvm-reviews.chandlerc.com/D736</a> for where this
is useful.
<a class="moz-txt-link-freetext" href="http://llvm-reviews.chandlerc.com/D783">http://llvm-reviews.chandlerc.com/D783</a>
Files:
include/llvm/ADT/APFloat.h
include/llvm/ADT/APInt.h
lib/Support/APFloat.cpp
Index: include/llvm/ADT/APFloat.h
===================================================================
--- include/llvm/ADT/APFloat.h
+++ include/llvm/ADT/APFloat.h
@@ -190,6 +190,10 @@
APFloat(const APFloat &);
~APFloat();
+ bool needsCleanup() const {
+ return partCount() > 1;
+ }
+
// Convenience "constructors"
static APFloat getZero(const fltSemantics &Sem, bool Negative = false) {
return APFloat(Sem, fcZero, Negative);
Index: include/llvm/ADT/APInt.h
===================================================================
--- include/llvm/ADT/APInt.h
+++ include/llvm/ADT/APInt.h
@@ -283,14 +283,18 @@
/// @brief Destructor.
~APInt() {
- if (!isSingleWord())
+ if (needsCleanup())
delete [] pVal;
}
/// Default constructor that creates an uninitialized APInt. This is useful
/// for object deserialization (pair this with the static method Read).
explicit APInt() : BitWidth(1) {}
+ bool needsCleanup() const {
+ return !isSingleWord();
+ }
+
/// Profile - Used to insert APInt objects, or objects that contain APInt
/// objects, into FoldingSets.
void Profile(FoldingSetNodeID& id) const;
Index: lib/Support/APFloat.cpp
===================================================================
--- lib/Support/APFloat.cpp
+++ lib/Support/APFloat.cpp
@@ -580,7 +580,7 @@
void
APFloat::freeSignificand()
{
- if (partCount() > 1)
+ if (needsCleanup())
delete [] significand.parts;
}
</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
</body>
</html>