[llvm] r222965 - Make MultiClass::DefPrototypes own their Records to fix memory leaks.

[Craig Topper]

On Wed, Dec 3, 2014 at 1:04 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Sat, Nov 29, 2014 at 4:19 PM, Craig Topper <craig.topper at gmail.com>
> wrote:
>
>> Author: ctopper
>> Date: Sat Nov 29 18:19:28 2014
>> New Revision: 222965
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=222965&view=rev
>> Log:
>> Make MultiClass::DefPrototypes own their Records to fix memory leaks.
>>
>> Modified:
>>     llvm/trunk/include/llvm/TableGen/Record.h
>>     llvm/trunk/lib/TableGen/TGParser.cpp
>>
>> Modified: llvm/trunk/include/llvm/TableGen/Record.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/TableGen/Record.h?rev=222965&r1=222964&r2=222965&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/TableGen/Record.h (original)
>> +++ llvm/trunk/include/llvm/TableGen/Record.h Sat Nov 29 18:19:28 2014
>> @@ -1663,7 +1663,7 @@ raw_ostream &operator<<(raw_ostream &OS,
>>
>>  struct MultiClass {
>>    Record Rec;  // Placeholder for template args and Name.
>> -  typedef std::vector<Record*> RecordVector;
>> +  typedef std::vector<std::unique_ptr<Record>> RecordVector;
>>
>
> Any chance of deque<Record> (or forward_list<Record> or list<Record>) here?
>

We don't always know at the pointer of record creation whether the record
will go in this list or to addDef. And the Record copy constructor alters
the record id. So it seemed weird to make a copy when we figure out where
it goes.


>
>
>>    RecordVector DefPrototypes;
>>
>>    void dump() const;
>>
>> Modified: llvm/trunk/lib/TableGen/TGParser.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/TableGen/TGParser.cpp?rev=222965&r1=222964&r2=222965&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/TableGen/TGParser.cpp (original)
>> +++ llvm/trunk/lib/TableGen/TGParser.cpp Sat Nov 29 18:19:28 2014
>> @@ -239,7 +239,7 @@ bool TGParser::AddSubMultiClass(MultiCla
>>        if (AddValue(NewDef.get(), SubMultiClass.RefRange.Start,
>> MCVals[i]))
>>          return true;
>>
>> -    CurMC->DefPrototypes.push_back(NewDef.release());
>> +    CurMC->DefPrototypes.push_back(std::move(NewDef));
>>    }
>>
>>    const std::vector<Init *> &SMCTArgs = SMC->Rec.getTemplateArgs();
>> @@ -274,7 +274,7 @@ bool TGParser::AddSubMultiClass(MultiCla
>>               jend = CurMC->DefPrototypes.end();
>>             j != jend;
>>             ++j) {
>> -        Record *Def = *j;
>> +        Record *Def = j->get();
>>
>>          if (SetValue(Def, SubMultiClass.RefRange.Start, SMCTArgs[i],
>>                       std::vector<unsigned>(),
>> @@ -1258,7 +1258,7 @@ Init *TGParser::ParseSimpleValue(Record
>>        // known before any use.
>>        NewRec->setResolveFirst(true);
>>        // Otherwise, we're inside a multiclass, add it to the multiclass.
>> -      CurMultiClass->DefPrototypes.push_back(NewRecOwner.release());
>> +      CurMultiClass->DefPrototypes.push_back(std::move(NewRecOwner));
>>
>>        // Copy the template arguments for the multiclass into the def.
>>        const std::vector<Init *> &TArgs =
>> @@ -2063,7 +2063,7 @@ bool TGParser::ParseDef(MultiClass *CurM
>>            == CurRec->getNameInit())
>>          return Error(DefLoc, "def '" + CurRec->getNameInitAsString() +
>>                       "' already defined in this multiclass!");
>> -    CurMultiClass->DefPrototypes.push_back(CurRecOwner.release());
>> +    CurMultiClass->DefPrototypes.push_back(std::move(CurRecOwner));
>>    } else if (ParseObjectBody(CurRec)) {
>>      return true;
>>    }
>> @@ -2507,7 +2507,7 @@ bool TGParser::ResolveMulticlassDef(Mult
>>          == CurRec->getNameInit())
>>        return Error(DefmPrefixLoc, "defm '" +
>> CurRec->getNameInitAsString() +
>>                     "' already defined in this multiclass!");
>> -  CurMultiClass->DefPrototypes.push_back(CurRec);
>> +
>> CurMultiClass->DefPrototypes.push_back(std::unique_ptr<Record>(CurRec));
>>
>>    // Copy the template arguments for the multiclass into the new def.
>>    const std::vector<Init *> &TA =
>> @@ -2570,7 +2570,7 @@ bool TGParser::ParseDefm(MultiClass *Cur
>>
>>      // Loop over all the def's in the multiclass, instantiating each one.
>>      for (unsigned i = 0, e = MC->DefPrototypes.size(); i != e; ++i) {
>> -      Record *DefProto = MC->DefPrototypes[i];
>> +      Record *DefProto = MC->DefPrototypes[i].get();
>>
>>        Record *CurRec = InstantiateMulticlassDef(*MC, DefProto,
>> DefmPrefix,
>>                                                  SMRange(DefmLoc,
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>


-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141203/8ad0de47/attachment.html>