Changeset 2c187378

Timestamp:

Oct 17, 2018, 4:16:18 PM (6 years ago)

Author:

Aaron Moss <a3moss@…>

Branches:

ADT, aaron-thesis, arm-eh, ast-experimental, cleanup-dtors, deferred_resn, enum, forall-pointer-decay, jacob/cs343-translation, jenkins-sandbox, master, new-ast, new-ast-unique-expr, no_list, persistent-indexer, pthread-emulation, qualifiedEnum

Children:

fbecee5

Parents:

da48183

Message:

Fix memory bugs in deferred resolution

Location:

src

Files:

• ResolvExpr/Alternative.cc (modified) (4 diffs)
• ResolvExpr/Alternative.h (modified) (3 diffs)
• ResolvExpr/AlternativeFinder.cc (modified) (11 diffs)
• ResolvExpr/ResolveAssertions.cc (modified) (3 diffs)
• Tuples/TupleAssignment.cc (modified) (1 diff)

src/ResolvExpr/Alternative.cc

@@ -20 +20 @@
 #include <utility>                       // for move
 
-#include "Common/utility.h"              // for maybeClone
+#include "Common/utility.h"              // for cloneAll
 #include "ResolvExpr/Cost.h"             // for Cost, Cost::zero, operator<<
 #include "ResolvExpr/TypeEnvironment.h"  // for TypeEnvironment
@@ -35 +35 @@
         Alternative::Alternative( const Alternative &o, Expression *expr, const Cost &cost ) 
         : cost( cost ), cvtCost( Cost::zero ), expr( expr ), env( o.env ), openVars( o.openVars ), 
-          need( o.need ) {}
+          need() { cloneAll( o.need, need ); }
 
         Alternative::Alternative( Expression *expr, const TypeEnvironment &env, 
-                const OpenVarSet& openVars, const AssertionList& need, const Cost& cost )
+                const OpenVarSet& openVars, const AssertionList& oneed, const Cost& cost )
         : cost( cost ), cvtCost( Cost::zero ), expr( expr ), env( env ), openVars( openVars ), 
-          need( need ) {}
+          need() { cloneAll( oneed, need ); }
 
         Alternative::Alternative( Expression *expr, const TypeEnvironment &env, 
-                const OpenVarSet& openVars, const AssertionList& need, const Cost& cost, 
+                const OpenVarSet& openVars, const AssertionList& oneed, const Cost& cost, 
                 const Cost &cvtCost )
         : cost( cost ), cvtCost( cvtCost ), expr( expr ), env( env ), openVars( openVars ), 
-          need( need ) {}
+          need() { cloneAll( oneed, need ); }
+        
+        Alternative::Alternative( Expression *expr, const TypeEnvironment &env, 
+                const OpenVarSet &openVars, const AssertionSet &oneed, const Cost &cost)
+        : cost( cost ), cvtCost( Cost::zero ), expr( expr ), env( env ), openVars( openVars ), 
+          need() { cloneAll( oneed, need ); }
+        
+        Alternative::Alternative( Expression *expr, const TypeEnvironment &env, 
+                const OpenVarSet &openVars, const AssertionSet &oneed, const Cost &cost, 
+                const Cost& cvtCost )
+        : cost( cost ), cvtCost( cvtCost ), expr( expr ), env( env ), openVars( openVars ), 
+          need() { cloneAll( oneed, need ); }
+        
+        Alternative::Alternative( Expression *expr, TypeEnvironment &&env, OpenVarSet &&openVars, 
+                AssertionSet &&needSet, const Cost &cost )
+        : cost( cost ), cvtCost( Cost::zero ), expr( expr ), env( std::move(env) ), 
+          openVars( std::move(openVars) ), need( needSet.begin(), needSet.end() ) {}
+        
+        Alternative::Alternative( Expression *expr, TypeEnvironment &&env, OpenVarSet &&openVars, 
+                AssertionSet &&needSet, const Cost &cost, const Cost &cvtCost )
+        : cost( cost ), cvtCost( cvtCost ), expr( expr ), env( std::move(env) ), 
+          openVars( std::move(openVars) ), need( needSet.begin(), needSet.end() ) {}
 
         Alternative::Alternative( const Alternative &other ) 
         : cost( other.cost ), cvtCost( other.cvtCost ), expr( maybeClone( other.expr ) ), 
-          env( other.env ), openVars( other.openVars ), need( other.need ) {}
+          env( other.env ), openVars( other.openVars ), need() { cloneAll( other.need, need ); }
 
         Alternative &Alternative::operator=( const Alternative &other ) {
@@ -60 +81 @@
                 env = other.env;
                 openVars = other.openVars;
-                need = other.need;
+                need.clear();
+                cloneAll( other.need, need );
                 return *this;
         }
@@ -67 +89 @@
         : cost( other.cost ), cvtCost( other.cvtCost ), expr( other.expr ), 
           env( std::move( other.env ) ), openVars( std::move( other.openVars ) ), 
-          need( std::move( other.need ) ) {
-                other.expr = nullptr;
-        }
+          need( std::move( other.need ) ) { other.expr = nullptr; }
 
         Alternative & Alternative::operator=( Alternative && other ) {

src/ResolvExpr/Alternative.h

@@ -22 +22 @@
 #include "TypeEnvironment.h"  // for TypeEnvironment, AssertionSetValue
 
+#include "Common/utility.h"   // for maybeClone
+
 class Expression;
 
@@ -35 +37 @@
                 AssertionItem( const AssertionSet::value_type& e ) : decl(e.first), info(e.second) {}
                 operator AssertionSet::value_type () const { return { decl, info }; }
+
+                // to support cloneAll
+                AssertionItem clone() const { return { maybeClone(decl), info }; }
         };
         /// A list of unresolved assertions
         using AssertionList = std::vector<AssertionItem>;
+
+        /// Clones an assertion list into an assertion set
+        static inline void cloneAll( const AssertionList& src, AssertionSet& dst ) {
+                for ( const AssertionItem& item : src ) {
+                        dst.emplace( maybeClone(item.decl), item.info );
+                }
+        }
+
+        /// Clones an assertion set into an assertion list
+        static inline void cloneAll( const AssertionSet& src, AssertionList& dst ) {
+                dst.reserve( dst.size() + src.size() );
+                for ( const auto& entry : src ) {
+                        dst.emplace_back( maybeClone(entry.first), entry.second );
+                }
+        }
+
+        /// Clones an assertion list into an assertion list
+        static inline void cloneAll( const AssertionList& src, AssertionList& dst ) {
+                dst.reserve( dst.size() + src.size() );
+                for ( const AssertionItem& item : src ) {
+                        dst.emplace_back( maybeClone(item.decl), item.info );
+                }
+        }
 
         /// One option for resolution of an expression
@@ -48 +76 @@
                 Alternative( Expression *expr, const TypeEnvironment &env, const OpenVarSet& openVars, 
                         const AssertionList& need, const Cost &cost, const Cost &cvtCost );
+                Alternative( Expression *expr, const TypeEnvironment &env, const OpenVarSet &openVars, 
+                        const AssertionSet &need, const Cost &cost);
+                Alternative( Expression *expr, const TypeEnvironment &env, const OpenVarSet &openVars, 
+                        const AssertionSet &need, const Cost &cost, const Cost& cvtCost );
+                Alternative( Expression *expr, TypeEnvironment &&env, OpenVarSet &&openVars, 
+                        AssertionSet &&need, const Cost &cost );
+                Alternative( Expression *expr, TypeEnvironment &&env, OpenVarSet &&openVars, 
+                        AssertionSet &&need, const Cost &cost, const Cost &cvtCost );
                 Alternative( const Alternative &other );
                 Alternative &operator=( const Alternative &other );

src/ResolvExpr/AlternativeFinder.cc

@@ -962 +962 @@
                 }
                 // build and validate new alternative
-                Alternative newAlt{ 
-                        appExpr, result.env, result.openVars, 
-                        AssertionList( result.need.begin(), result.need.end() ), cost };
+                Alternative newAlt{ appExpr, result.env, result.openVars, result.need, cost };
                 PRINT(
                         std::cerr << "instantiate function success: " << appExpr << std::endl;
@@ -1330 +1328 @@
                                 Alternative newAlt{ 
                                         restructureCast( alt.expr->clone(), toType, castExpr->isGenerated ), 
-                                        alt.env, openVars, 
-                                        AssertionList( needAssertions.begin(), needAssertions.end() ), alt.cost, 
-                                        thisCost };
+                                        alt.env, openVars, needAssertions, alt.cost, thisCost };
                                 inferParameters( needAssertions, haveAssertions, newAlt, openVars,
                                         back_inserter( candidates ) );
@@ -1524 +1520 @@
                                 Expression * newExpr = data.combine( cost );
                                 alternatives.push_back( Alternative{ 
-                                        new AttrExpr{ newExpr, argType->clone() }, env, OpenVarSet{}, AssertionList{},  
-                                        Cost::zero, cost } );
+                                        new AttrExpr{ newExpr, argType->clone() }, env, OpenVarSet{}, 
+                                        AssertionList{}, Cost::zero, cost } );
                                 for ( DeclarationWithType * retVal : function->returnVals ) {
                                         alternatives.back().expr->result = retVal->get_type()->clone();
@@ -1584 +1580 @@
                                 OpenVarSet openVars{ first.openVars };
                                 mergeOpenVars( openVars, second.openVars );
-                                AssertionList need{ first.need };
-                                need.insert( need.end(), second.need.begin(), second.need.end() );
+                                AssertionSet need;
+                                cloneAll( first.need, need );
+                                cloneAll( second.need, need );
 
                                 LogicalExpr *newExpr = new LogicalExpr{ 
                                         first.expr->clone(), second.expr->clone(), logicalExpr->get_isAnd() };
                                 alternatives.push_back( Alternative{ 
-                                        newExpr, compositeEnv, openVars, need, first.cost + second.cost } );
+                                        newExpr, std::move(compositeEnv), std::move(openVars), 
+                                        AssertionList( need.begin(), need.end() ), first.cost + second.cost } );
                         }
                 }
@@ -1617 +1615 @@
                                         mergeOpenVars( openVars, second.openVars );
                                         mergeOpenVars( openVars, third.openVars );
-                                        AssertionSet needAssertions( first.need.begin(), first.need.end() );
-                                        needAssertions.insert( second.need.begin(), second.need.end() );
-                                        needAssertions.insert( third.need.begin(), third.need.end() );
-                                        AssertionSet haveAssertions;
+                                        AssertionSet need;
+                                        cloneAll( first.need, need );
+                                        cloneAll( second.need, need );
+                                        cloneAll( third.need, need );
+                                        AssertionSet have;
                                         
                                         // unify true and false types, then infer parameters to produce new alternatives
                                         Type* commonType = nullptr;
                                         if ( unify( second.expr->result, third.expr->result, compositeEnv, 
-                                                        needAssertions, haveAssertions, openVars, indexer, commonType ) ) {
+                                                        need, have, openVars, indexer, commonType ) ) {
                                                 ConditionalExpr *newExpr = new ConditionalExpr{ 
                                                         first.expr->clone(), second.expr->clone(), third.expr->clone() };
@@ -1637 +1636 @@
                                                 // output alternative
                                                 Alternative newAlt{ 
-                                                        newExpr, compositeEnv, openVars, 
-                                                        AssertionList( needAssertions.begin(), needAssertions.end() ), cost };
-                                                inferParameters( needAssertions, haveAssertions, newAlt, openVars, back_inserter( alternatives ) );
+                                                        newExpr, std::move(compositeEnv), std::move(openVars), 
+                                                        AssertionList( need.begin(), need.end() ), cost };
+                                                inferParameters( need, have, newAlt, openVars, back_inserter( alternatives ) );
                                         } // if
                                 } // for
@@ -1672 +1671 @@
                                 OpenVarSet openVars{ first.openVars };
                                 mergeOpenVars( openVars, second.openVars );
-                                AssertionSet needAssertions( first.need.begin(), first.need.end() );
-                                needAssertions.insert( second.need.begin(), second.need.end() );
-                                AssertionSet haveAssertions;
+                                AssertionSet need;
+                                cloneAll( first.need, need );
+                                cloneAll( second.need, need );
+                                AssertionSet have;
 
                                 Type* commonType = nullptr;
-                                if ( unify( first.expr->result, second.expr->result, compositeEnv, needAssertions, 
-                                                haveAssertions, openVars, indexer, commonType ) ) {
+                                if ( unify( first.expr->result, second.expr->result, compositeEnv, need, have, 
+                                                openVars, indexer, commonType ) ) {
                                         RangeExpr * newExpr = 
                                                 new RangeExpr{ first.expr->clone(), second.expr->clone() };
                                         newExpr->result = commonType ? commonType : first.expr->result->clone();
                                         Alternative newAlt{ 
-                                                newExpr, compositeEnv, openVars, 
-                                                AssertionList( needAssertions.begin(), needAssertions.end() ), 
-                                                first.cost + second.cost };
-                                        inferParameters( needAssertions, haveAssertions, newAlt, openVars, back_inserter( alternatives ) );
+                                                newExpr, std::move(compositeEnv), std::move(openVars), 
+                                                AssertionList( need.begin(), need.end() ), first.cost + second.cost };
+                                        inferParameters( need, have, newAlt, openVars, back_inserter( alternatives ) );
                                 } // if
                         } // for
@@ -1709 +1708 @@
                                 compositeEnv.simpleCombine( alt.env );
                                 mergeOpenVars( openVars, alt.openVars );
-                                need.insert( alt.need.begin(), alt.need.end() );
+                                cloneAll( alt.need, need );
                         }
                         
                         alternatives.push_back( Alternative{ 
-                                new TupleExpr{ exprs }, compositeEnv, openVars, 
+                                new TupleExpr{ exprs }, std::move(compositeEnv), std::move(openVars), 
                                 AssertionList( need.begin(), need.end() ), sumCost( alts ) } );
                 } // for
@@ -1781 +1780 @@
                         for ( Alternative & alt : finder.get_alternatives() ) {
                                 TypeEnvironment newEnv( alt.env );
-                                AssertionSet needAssertions( alt.need.begin(), alt.need.end() );
-                                AssertionSet haveAssertions;
+                                AssertionSet need;
+                                cloneAll( alt.need, need );
+                                AssertionSet have;
                                 OpenVarSet openVars( alt.openVars );  
                                 // xxx - find things in env that don't have a "representative type" and claim 
@@ -1801 +1801 @@
                                 
                                 // unification run for side-effects
-                                unify( toType, alt.expr->result, newEnv, needAssertions, haveAssertions, openVars, 
-                                        indexer );
+                                unify( toType, alt.expr->result, newEnv, need, have, openVars, indexer );
                                 // xxx - do some inspecting on this line... why isn't result bound to initAlt.type?
 
@@ -1812 +1811 @@
                                                 new InitExpr{ 
                                                         restructureCast( alt.expr->clone(), toType, true ), initAlt.designation->clone() }, 
-                                                newEnv, openVars, 
-                                                AssertionList( needAssertions.begin(), needAssertions.end() ), 
-                                                alt.cost, thisCost };
-                                        inferParameters( needAssertions, haveAssertions, newAlt, openVars, back_inserter( candidates ) );
+                                                std::move(newEnv), std::move(openVars), 
+                                                AssertionList( need.begin(), need.end() ), alt.cost, thisCost };
+                                        inferParameters( need, have, newAlt, openVars, back_inserter( candidates ) );
                                 }
                         }

src/ResolvExpr/ResolveAssertions.cc

@@ -166 +166 @@
 
                 Expression* varExpr = match.cdata.combine( alt.cvtCost );
-                varExpr->result = match.adjType;
+                delete varExpr->result;
+                varExpr->result = match.adjType->clone();
 
                 // follow the current assertion's ID chain to find the correct set of inferred parameters 
@@ -177 +178 @@
 
                 (*inferParams)[ decl->get_uniqueId() ] = ParamEntry{
-                                candidate->get_uniqueId(), match.adjType, decl->get_type(), varExpr };
+                                candidate->get_uniqueId(), match.adjType, decl->get_type()->clone(), varExpr };
         }
 
@@ -215 +216 @@
                                 matches.emplace_back( cdata, adjType, std::move(newEnv), std::move(have), 
                                         std::move(newNeed), std::move(newOpenVars) );
+                        } else {
+                                delete adjType;
                         }
                 }

src/Tuples/TupleAssignment.cc

@@ -72 +72 @@
                                 compositeEnv.simpleCombine( alt.env );
                                 ResolvExpr::mergeOpenVars( openVars, alt.openVars );
-                                need.insert( alt.need.begin(), alt.need.end() );
+                                cloneAll( alt.need, need );
                         }