Changes in src/InitTweak/FixInit.cc [1bc749f:e41306d]

File:

• src/InitTweak/FixInit.cc (modified) (28 diffs)

src/InitTweak/FixInit.cc

@@ -54 +54 @@
 #include "SynTree/Type.h"              // for Type, Type::StorageClasses
 #include "SynTree/TypeSubstitution.h"  // for TypeSubstitution, operator<<
-#include "SynTree/VarExprReplacer.h"   // for VarExprReplacer
 #include "SynTree/Visitor.h"           // for acceptAll, maybeAccept
 
@@ -94 +93 @@
                         /// true if type does not need to be copy constructed to ensure correctness
                         bool skipCopyConstruct( Type * type );
-                        void copyConstructArg( Expression *& arg, ImplicitCopyCtorExpr * impCpCtorExpr );
+                        void copyConstructArg( Expression *& arg, ImplicitCopyCtorExpr * impCpCtorExpr, Type * formal );
                         void destructRet( ObjectDecl * ret, ImplicitCopyCtorExpr * impCpCtorExpr );
 
@@ -159 +158 @@
                         using Parent::previsit;
 
+                        void previsit( ObjectDecl * objDecl );
                         void previsit( FunctionDecl * funcDecl );
 
+                        void previsit( CompoundStmt * compoundStmt );
+                        void postvisit( CompoundStmt * compoundStmt );
+                        void previsit( ReturnStmt * returnStmt );
                         void previsit( BranchStmt * stmt );
                 private:
@@ -176 +179 @@
 
                         DeclarationWithType * postmutate( ObjectDecl *objDecl );
-
-                        DeclarationWithType * getDtorFunc( ObjectDecl * objDecl, Statement * dtor );
 
                         std::list< Declaration * > staticDtorDecls;
@@ -213 +214 @@
                         void emit( CodeLocation, const Params &... params );
 
-                        FunctionDecl * function = 0;
+                        FunctionDecl * function = nullptr;
                         std::set< DeclarationWithType * > unhandled;
                         std::map< DeclarationWithType *, CodeLocation > usedUninit;
-                        ObjectDecl * thisParam = 0;
+                        ObjectDecl * thisParam = nullptr;
                         bool isCtor = false; // true if current function is a constructor
-                        StructDecl * structDecl = 0;
+                        StructDecl * structDecl = nullptr;
                 };
 
@@ -259 +260 @@
 
                 GenStructMemberCalls::generate( translationUnit );
+
                 // xxx - ctor expansion currently has to be after FixCopyCtors, because there is currently a
                 // hack in the way untyped assignments are generated, where the first argument cannot have
@@ -288 +290 @@
                         for ( std::list< Declaration * >::iterator i = translationUnit.begin(); i != translationUnit.end(); ++i ) {
                                 try {
-                                        *i = maybeMutate( *i, fixer );
+                                        maybeMutate( *i, fixer );
                                         translationUnit.splice( i, fixer.pass.staticDtorDecls );
                                 } catch( SemanticError &e ) {
@@ -322 +324 @@
 
                 Expression * InsertImplicitCalls::postmutate( ApplicationExpr * appExpr ) {
-                        assert( appExpr );
-
                         if ( VariableExpr * function = dynamic_cast< VariableExpr * > ( appExpr->get_function() ) ) {
-                                if ( LinkageSpec::isBuiltin( function->get_var()->get_linkage() ) ) {
+                                if ( function->var->linkage.is_builtin ) {
                                         // optimization: don't need to copy construct in order to call intrinsic functions
                                         return appExpr;
@@ -331 +331 @@
                                         FunctionType * ftype = dynamic_cast< FunctionType * >( GenPoly::getFunctionType( funcDecl->get_type() ) );
                                         assertf( ftype, "Function call without function type: %s", toString( funcDecl ).c_str() );
-                                        if ( CodeGen::isConstructor( funcDecl->get_name() ) && ftype->get_parameters().size() == 2 ) {
-                                                Type * t1 = getPointerBase( ftype->get_parameters().front()->get_type() );
-                                                Type * t2 = ftype->get_parameters().back()->get_type();
+                                        if ( CodeGen::isConstructor( funcDecl->get_name() ) && ftype->parameters.size() == 2 ) {
+                                                Type * t1 = getPointerBase( ftype->parameters.front()->get_type() );
+                                                Type * t2 = ftype->parameters.back()->get_type();
                                                 assert( t1 );
 
@@ -366 +366 @@
                         ImplicitCtorDtorStmt * stmt = genCtorDtor( fname, var, cpArg );
                         ExprStmt * exprStmt = strict_dynamic_cast< ExprStmt * >( stmt->get_callStmt() );
-                        Expression * untyped = exprStmt->get_expr();
+                        Expression * resolved = exprStmt->expr;
+                        exprStmt->expr = nullptr; // take ownership of expr
 
                         // resolve copy constructor
                         // should only be one alternative for copy ctor and dtor expressions, since all arguments are fixed
                         // (VariableExpr and already resolved expression)
-                        CP_CTOR_PRINT( std::cerr << "ResolvingCtorDtor " << untyped << std::endl; )
-                        Expression * resolved = ResolvExpr::findVoidExpression( untyped, indexer );
+                        CP_CTOR_PRINT( std::cerr << "ResolvingCtorDtor " << resolved << std::endl; )
+                        ResolvExpr::findVoidExpression( resolved, indexer );
                         assert( resolved );
                         if ( resolved->get_env() ) {
@@ -380 +381 @@
                                 resolved->set_env( nullptr );
                         } // if
-
                         delete stmt;
                         return resolved;
                 }
 
-                void ResolveCopyCtors::copyConstructArg( Expression *& arg, ImplicitCopyCtorExpr * impCpCtorExpr ) {
+                void ResolveCopyCtors::copyConstructArg( Expression *& arg, ImplicitCopyCtorExpr * impCpCtorExpr, Type * formal ) {
                         static UniqueName tempNamer("_tmp_cp");
                         assert( env );
                         CP_CTOR_PRINT( std::cerr << "Type Substitution: " << *env << std::endl; )
-                        assert( arg->has_result() );
-                        Type * result = arg->get_result();
+                        assert( arg->result );
+                        Type * result = arg->result;
                         if ( skipCopyConstruct( result ) ) return; // skip certain non-copyable types
 
-                        // type may involve type variables, so apply type substitution to get temporary variable's actual type
+                        // type may involve type variables, so apply type substitution to get temporary variable's actual type.
+                        // Use applyFree so that types bound in function pointers are not substituted, e.g. in forall(dtype T) void (*)(T).
                         result = result->clone();
-                        env->apply( result );
+                        env->applyFree( result );
                         ObjectDecl * tmp = ObjectDecl::newObject( "__tmp", result, nullptr );
                         tmp->get_type()->set_const( false );
@@ -406 +407 @@
                                 // if the chosen constructor is intrinsic, the copy is unnecessary, so
                                 // don't create the temporary and don't call the copy constructor
-                                VariableExpr * function = dynamic_cast< VariableExpr * >( appExpr->get_function() );
-                                assert( function );
-                                if ( function->get_var()->get_linkage() == LinkageSpec::Intrinsic ) return;
+                                VariableExpr * function = strict_dynamic_cast< VariableExpr * >( appExpr->function );
+                                if ( function->var->linkage == LinkageSpec::Intrinsic ) {
+                                        // arguments that need to be boxed need a temporary regardless of whether the copy constructor is intrinsic,
+                                        // so that the object isn't changed inside of the polymorphic function
+                                        if ( ! GenPoly::needsBoxing( formal, result, impCpCtorExpr->callExpr, env ) ) return;
+                                }
                         }
 
@@ -416 +420 @@
                         // replace argument to function call with temporary
                         arg = new CommaExpr( cpCtor, new VariableExpr( tmp ) );
-                        impCpCtorExpr->get_tempDecls().push_back( tmp );
-                        impCpCtorExpr->get_dtors().push_front( makeCtorDtor( "^?{}", tmp ) );
+                        impCpCtorExpr->tempDecls.push_back( tmp );
+                        impCpCtorExpr->dtors.push_front( makeCtorDtor( "^?{}", tmp ) );
                 }
 
@@ -427 +431 @@
                         CP_CTOR_PRINT( std::cerr << "ResolveCopyCtors: " << impCpCtorExpr << std::endl; )
 
-                        ApplicationExpr * appExpr = impCpCtorExpr->get_callExpr();
+                        ApplicationExpr * appExpr = impCpCtorExpr->callExpr;
 
                         // take each argument and attempt to copy construct it.
-                        for ( Expression * & arg : appExpr->get_args() ) {
-                                copyConstructArg( arg, impCpCtorExpr );
+                        FunctionType * ftype = GenPoly::getFunctionType( appExpr->function->result );
+                        assert( ftype );
+                        auto & params = ftype->parameters;
+                        auto iter = params.begin();
+                        for ( Expression * & arg : appExpr->args ) {
+                                Type * formal = nullptr;
+                                if ( iter != params.end() ) {
+                                        DeclarationWithType * param = *iter++;
+                                        formal = param->get_type();
+                                }
+
+                                copyConstructArg( arg, impCpCtorExpr, formal );
                         } // for
 
@@ -437 +451 @@
                         // initialized with the return value and is destructed later
                         // xxx - handle named return values?
-                        Type * result = appExpr->get_result();
+                        Type * result = appExpr->result;
                         if ( ! result->isVoid() ) {
                                 static UniqueName retNamer("_tmp_cp_ret");
@@ -443 +457 @@
                                 env->apply( result );
                                 ObjectDecl * ret = ObjectDecl::newObject( retNamer.newName(), result, nullptr );
-                                ret->get_type()->set_const( false );
-                                impCpCtorExpr->get_returnDecls().push_back( ret );
+                                ret->type->set_const( false );
+                                impCpCtorExpr->returnDecls.push_back( ret );
                                 CP_CTOR_PRINT( std::cerr << "makeCtorDtor for a return" << std::endl; )
                                 if ( ! dynamic_cast< ReferenceType * >( result ) ) {
@@ -551 +565 @@
                                 Expression * retExpr = new CommaExpr( assign, new VariableExpr( returnDecl ) );
                                 // move env from callExpr to retExpr
-                                retExpr->set_env( callExpr->get_env() );
-                                callExpr->set_env( nullptr );
+                                std::swap( retExpr->env, callExpr->env );
                                 return retExpr;
                         } else {
@@ -623 +636 @@
                 }
 
-                DeclarationWithType * FixInit::getDtorFunc( ObjectDecl * objDecl, Statement * dtor ) {
-                        if ( dynamic_cast< ExprStmt * >( dtor ) ) {
-                                if ( DeclarationWithType * func = getFunction( getCtorDtorCall( dtor ) ) ) {
-                                        // cleanup argument must be a function, not an object (including function pointer)
-                                        if ( FunctionDecl * dtorFunc = dynamic_cast< FunctionDecl * > ( func ) ) {
-                                                if ( dtorFunc->type->forall.empty() ) {
-                                                        // simple case where the destructor is a monomorphic function call - can simply
-                                                        // use that function as the cleanup function.
-                                                        delete dtor;
-                                                        return func;
-                                                }
-                                        }
-                                }
-                        }
-
-                        // otherwise the cleanup is more complicated - need to build a single argument cleanup function that
-                        // wraps the more complicated code.
-                        static UniqueName dtorNamer( "__cleanup_dtor" );
-                        FunctionDecl * dtorFunc = FunctionDecl::newFunction( dtorNamer.newName(), SymTab::genDefaultType( objDecl->type ), new CompoundStmt( noLabels ) );
-                        stmtsToAddBefore.push_back( new DeclStmt( noLabels, dtorFunc ) );
-
-                        // the original code contains uses of objDecl - replace them with the newly generated 'this' parameter.
-                        ObjectDecl * thisParam = getThisParam( dtorFunc->type );
-                        VarExprReplacer::replace( dtor, { std::make_pair( objDecl, thisParam ) } );
-                        dtorFunc->statements->push_back( dtor );
-
-                        return dtorFunc;
-                }
-
                 DeclarationWithType * FixInit::postmutate( ObjectDecl *objDecl ) {
                         // since this removes the init field from objDecl, it must occur after children are mutated (i.e. postmutate)
@@ -750 +734 @@
                                         } else {
                                                 ImplicitCtorDtorStmt * implicit = strict_dynamic_cast< ImplicitCtorDtorStmt * > ( ctor );
-                                                ExprStmt * ctorStmt = dynamic_cast< ExprStmt * >( implicit->get_callStmt() );
+                                                ExprStmt * ctorStmt = dynamic_cast< ExprStmt * >( implicit->callStmt );
                                                 ApplicationExpr * ctorCall = nullptr;
-                                                if ( ctorStmt && (ctorCall = isIntrinsicCallExpr( ctorStmt->get_expr() )) && ctorCall->get_args().size() == 2 ) {
+                                                if ( ctorStmt && (ctorCall = isIntrinsicCallExpr( ctorStmt->expr )) && ctorCall->get_args().size() == 2 ) {
                                                         // clean up intrinsic copy constructor calls by making them into SingleInits
-                                                        objDecl->set_init( new SingleInit( ctorCall->get_args().back() ) );
-                                                        ctorCall->get_args().pop_back();
+                                                        Expression * ctorArg = ctorCall->args.back();
+                                                        std::swap( ctorArg->env, ctorCall->env );
+                                                        objDecl->init = new SingleInit( ctorArg );
+
+                                                        ctorCall->args.pop_back();
                                                 } else {
                                                         stmtsToAddAfter.push_back( ctor );
-                                                        objDecl->set_init( nullptr );
-                                                        ctorInit->set_ctor( nullptr );
-                                                }
-
-                                                Statement * dtor = ctorInit->dtor;
-                                                if ( dtor ) {
-                                                        ImplicitCtorDtorStmt * implicit = strict_dynamic_cast< ImplicitCtorDtorStmt * >( dtor );
-                                                        Statement * dtorStmt = implicit->callStmt;
-                                                        // don't need to call intrinsic dtor, because it does nothing, but
-                                                        // non-intrinsic dtors must be called
-                                                        if ( ! isIntrinsicSingleArgCallStmt( dtorStmt ) ) {
-                                                                // set dtor location to the object's location for error messages
-                                                                DeclarationWithType * dtorFunc = getDtorFunc( objDecl, dtorStmt );
-                                                                objDecl->attributes.push_back( new Attribute( "cleanup", { new VariableExpr( dtorFunc ) } ) );
-                                                                // objDecl->attributes.push_back( new Attribute( "cleanup", { new NameExpr( dtorFunc->name ) } ) );
-                                                                ctorInit->dtor = nullptr;
-                                                        } // if
+                                                        objDecl->init = nullptr;
+                                                        ctorInit->ctor = nullptr;
                                                 }
                                         } // if
-                                } else if ( Initializer * init = ctorInit->get_init() ) {
-                                        objDecl->set_init( init );
-                                        ctorInit->set_init( nullptr );
+                                } else if ( Initializer * init = ctorInit->init ) {
+                                        objDecl->init = init;
+                                        ctorInit->init = nullptr;
                                 } else {
                                         // no constructor and no initializer, which is okay
-                                        objDecl->set_init( nullptr );
+                                        objDecl->init = nullptr;
                                 } // if
                                 delete ctorInit;
@@ -818 +790 @@
 
 
+                template<typename Iterator, typename OutputIterator>
+                void insertDtors( Iterator begin, Iterator end, OutputIterator out ) {
+                        for ( Iterator it = begin ; it != end ; ++it ) {
+                                // extract destructor statement from the object decl and insert it into the output. Note that this is
+                                // only called on lists of non-static objects with implicit non-intrinsic dtors, so if the user manually
+                                // calls an intrinsic dtor then the call must (and will) still be generated since the argument may
+                                // contain side effects.
+                                ObjectDecl * objDecl = *it;
+                                ConstructorInit * ctorInit = dynamic_cast< ConstructorInit * >( objDecl->get_init() );
+                                assert( ctorInit && ctorInit->get_dtor() );
+                                *out++ = ctorInit->get_dtor()->clone();
+                        } // for
+                }
+
+                void InsertDtors::previsit( ObjectDecl * objDecl ) {
+                        // remember non-static destructed objects so that their destructors can be inserted later
+                        if ( ! objDecl->get_storageClasses().is_static ) {
+                                if ( ConstructorInit * ctorInit = dynamic_cast< ConstructorInit * >( objDecl->get_init() ) ) {
+                                        // a decision should have been made by the resolver, so ctor and init are not both non-NULL
+                                        assert( ! ctorInit->get_ctor() || ! ctorInit->get_init() );
+                                        Statement * dtor = ctorInit->get_dtor();
+                                        // don't need to call intrinsic dtor, because it does nothing, but
+                                        // non-intrinsic dtors must be called
+                                        if ( dtor && ! isIntrinsicSingleArgCallStmt( dtor ) ) {
+                                                // set dtor location to the object's location for error messages
+                                                ctorInit->dtor->location = objDecl->location;
+                                                reverseDeclOrder.front().push_front( objDecl );
+                                        } // if
+                                } // if
+                        } // if
+                }
+
                 void InsertDtors::previsit( FunctionDecl * funcDecl ) {
                         // each function needs to have its own set of labels
                         GuardValue( labelVars );
                         labelVars.clear();
+                        // LabelFinder does not recurse into FunctionDecl, so need to visit
+                        // its children manually.
                         maybeAccept( funcDecl->type, finder );
                         maybeAccept( funcDecl->statements, finder );
 
                         // all labels for this function have been collected, insert destructors as appropriate via implicit recursion.
+                }
+
+                void InsertDtors::previsit( CompoundStmt * compoundStmt ) {
+                        // visit statements - this will also populate reverseDeclOrder list.  don't want to dump all destructors
+                        // when block is left, just the destructors associated with variables defined in this block, so push a new
+                        // list to the top of the stack so that we can differentiate scopes
+                        reverseDeclOrder.push_front( OrderedDecls() );
+                        Parent::previsit( compoundStmt );
+                }
+
+                void InsertDtors::postvisit( CompoundStmt * compoundStmt ) {
+                        // add destructors for the current scope that we're exiting, unless the last statement is a return, which
+                        // causes unreachable code warnings
+                        std::list< Statement * > & statements = compoundStmt->get_kids();
+                        if ( ! statements.empty() && ! dynamic_cast< ReturnStmt * >( statements.back() ) ) {
+                                insertDtors( reverseDeclOrder.front().begin(), reverseDeclOrder.front().end(), back_inserter( statements ) );
+                        }
+                        reverseDeclOrder.pop_front();
+                }
+
+                void InsertDtors::previsit( ReturnStmt * ) {
+                        // return exits all scopes, so dump destructors for all scopes
+                        for ( OrderedDecls & od : reverseDeclOrder ) {
+                                insertDtors( od.begin(), od.end(), back_inserter( stmtsToAddBefore ) );
+                        } // for
                 }
 
@@ -855 +886 @@
                         if ( ! diff.empty() ) {
                                 throw SemanticError( std::string("jump to label '") + stmt->get_target().get_name() + "' crosses initialization of " + (*diff.begin())->get_name() + " ", stmt );
+                        } // if
+                        // S_G-S_L results in set of objects that must be destructed
+                        diff.clear();
+                        std::set_difference( curVars.begin(), curVars.end(), lvars.begin(), lvars.end(), std::inserter( diff, diff.end() ) );
+                        DTOR_PRINT(
+                                std::cerr << "S_G-S_L = " << printSet( diff ) << std::endl;
+                        )
+                        if ( ! diff.empty() ) {
+                                // create an auxilliary set for fast lookup -- can't make diff a set, because diff ordering should be consistent for error messages.
+                                std::unordered_set<ObjectDecl *> needsDestructor( diff.begin(), diff.end() );
+
+                                // go through decl ordered list of objectdecl. for each element that occurs in diff, output destructor
+                                OrderedDecls ordered;
+                                for ( OrderedDecls & rdo : reverseDeclOrder ) {
+                                        // add elements from reverseDeclOrder into ordered if they occur in diff - it is key that this happens in reverse declaration order.
+                                        copy_if( rdo.begin(), rdo.end(), back_inserter( ordered ), [&]( ObjectDecl * objDecl ) { return needsDestructor.count( objDecl ); } );
+                                } // for
+                                insertDtors( ordered.begin(), ordered.end(), back_inserter( stmtsToAddBefore ) );
                         } // if
                 }
@@ -880 +929 @@
                 }
 
+                void addIds( SymTab::Indexer & indexer, const std::list< DeclarationWithType * > & decls ) {
+                        for ( auto d : decls ) {
+                                indexer.addId( d );
+                        }
+                }
+
+                void addTypes( SymTab::Indexer & indexer, const std::list< TypeDecl * > & tds ) {
+                        for ( auto td : tds ) {
+                                indexer.addType( td );
+                                addIds( indexer, td->assertions );
+                        }
+                }
+
                 void GenStructMemberCalls::previsit( FunctionDecl * funcDecl ) {
-                        GuardValue( funcDecl );
+                        GuardValue( function );
                         GuardValue( unhandled );
                         GuardValue( usedUninit );
@@ -914 +976 @@
                 }
 
-                void addIds( SymTab::Indexer & indexer, const std::list< DeclarationWithType * > & decls ) {
-                        for ( auto d : decls ) {
-                                indexer.addId( d );
-                        }
-                }
-
-                void addTypes( SymTab::Indexer & indexer, const std::list< TypeDecl * > & tds ) {
-                        for ( auto td : tds ) {
-                                indexer.addType( td );
-                                addIds( indexer, td->assertions );
-                        }
-                }
-
                 void GenStructMemberCalls::postvisit( FunctionDecl * funcDecl ) {
                         // remove the unhandled objects from usedUninit, because a call is inserted
@@ -1079 +1128 @@
                 }
 
-                DeclarationWithType * MutatingResolver::mutate( ObjectDecl *objectDecl ) {
+                DeclarationWithType * MutatingResolver::mutate( ObjectDecl * objectDecl ) {
                         // add object to the indexer assumes that there will be no name collisions
                         // in generated code. If this changes, add mutate methods for entities with
@@ -1087 +1136 @@
                 }
 
-                Expression* MutatingResolver::mutate( UntypedExpr *untypedExpr ) {
-                        return strict_dynamic_cast< ApplicationExpr * >( ResolvExpr::findVoidExpression( untypedExpr, indexer ) );
+                Expression * MutatingResolver::mutate( UntypedExpr * untypedExpr ) {
+                        Expression * newExpr = untypedExpr;
+                        ResolvExpr::findVoidExpression( newExpr, indexer );
+                        return newExpr;
                 }
 
@@ -1094 +1145 @@
                         static UniqueName tempNamer( "_tmp_ctor_expr" );
                         // xxx - is the size check necessary?
-                        assert( ctorExpr->has_result() && ctorExpr->get_result()->size() == 1 );
+                        assert( ctorExpr->result && ctorExpr->get_result()->size() == 1 );
 
                         // xxx - ideally we would reuse the temporary generated from the copy constructor passes from within firstArg if it exists and not generate a temporary if it's unnecessary.
@@ -1113 +1164 @@
 
                         // resolve assignment and dispose of new env
-                        Expression * resolvedAssign = ResolvExpr::findVoidExpression( assign, indexer );
-                        delete resolvedAssign->env;
-                        resolvedAssign->env = nullptr;
-                        delete assign;
+                        ResolvExpr::findVoidExpression( assign, indexer );
+                        delete assign->env;
+                        assign->env = nullptr;
 
                         // for constructor expr:
@@ -1125 +1175 @@
                         //   T & tmp;
                         //   &tmp = &x, ?{}(tmp), tmp
-                        CommaExpr * commaExpr = new CommaExpr( resolvedAssign, new CommaExpr( callExpr, new VariableExpr( tmp ) ) );
+                        CommaExpr * commaExpr = new CommaExpr( assign, new CommaExpr( callExpr, new VariableExpr( tmp ) ) );
                         commaExpr->set_env( env );
                         return commaExpr;