Index: src/GenPoly/Box.cc =================================================================== --- src/GenPoly/Box.cc (revision fc638d2176b72a88b406e73af7f7f9e548ae05f7) +++ src/GenPoly/Box.cc (revision cce94296a61149881755d331f2f292f92c7f6c5c) @@ -131,6 +131,7 @@ ScopedMap< std::string, DeclarationWithType* > adapters; ///< Set of adapter functions in the current scope + std::map< ApplicationExpr *, Expression * > retVals; + DeclarationWithType *retval; - bool useRetval; UniqueName tempNamer; }; @@ -497,5 +498,5 @@ } - Pass1::Pass1() : useRetval( false ), tempNamer( "_temp" ) {} + Pass1::Pass1() : tempNamer( "_temp" ) {} /// Returns T if the given declaration is a function with parameter (T*) for some TypeInstType T, NULL otherwise @@ -667,9 +668,8 @@ DeclarationWithType *oldRetval = retval; - bool oldUseRetval = useRetval; // process polymorphic return value - retval = 0; - if ( isDynRet( functionDecl->get_functionType() ) && functionDecl->get_linkage() == LinkageSpec::Cforall ) { + retval = nullptr; + if ( isDynRet( functionDecl->get_functionType() ) && functionDecl->get_linkage() != LinkageSpec::C ) { retval = functionDecl->get_functionType()->get_returnVals().front(); @@ -700,5 +700,5 @@ if ( adapters.find( mangleName ) == adapters.end() ) { std::string adapterName = makeAdapterName( mangleName ); - adapters.insert( std::pair< std::string, DeclarationWithType *>( mangleName, new ObjectDecl( adapterName, DeclarationNode::NoStorageClass, LinkageSpec::C, 0, new PointerType( Type::Qualifiers(), makeAdapterType( *funType, scopeTyVars ) ), 0 ) ) ); + adapters.insert( std::pair< std::string, DeclarationWithType *>( mangleName, new ObjectDecl( adapterName, DeclarationNode::NoStorageClass, LinkageSpec::C, nullptr, new PointerType( Type::Qualifiers(), makeAdapterType( *funType, scopeTyVars ) ), nullptr ) ) ); } // if } // for @@ -712,5 +712,4 @@ dtorOps.endScope(); retval = oldRetval; - useRetval = oldUseRetval; doEndScope(); } // if @@ -724,8 +723,21 @@ Expression *Pass1::mutate( CommaExpr *commaExpr ) { - bool oldUseRetval = useRetval; - useRetval = false; + // Attempting to find application expressions that were mutated by the copy constructor passes + // to use an explicit return variable, so that the variable can be reused as a parameter to the + // call rather than creating a new temp variable. Previously this step was an optimization, but + // with the introduction of tuples and UniqueExprs, it is necessary to ensure that they use the same variable. + // Essentially, looking for pattern: (x=f(...), x) + // To compound the issue, the right side can be *x, etc. because of lvalue-returning functions + if ( UntypedExpr * assign = dynamic_cast< UntypedExpr * >( commaExpr->get_arg1() ) ) { + if ( InitTweak::isAssignment( InitTweak::getFunctionName( assign ) ) ) { + assert( assign->get_args().size() == 2 ); + if ( ApplicationExpr * appExpr = dynamic_cast< ApplicationExpr * > ( assign->get_args().back() ) ) { + // first argument is assignable, so it must be an lvalue, so it should be legal to take its address. + retVals[appExpr] = assign->get_args().front(); + } + } + } + commaExpr->set_arg1( maybeMutate( commaExpr->get_arg1(), *this ) ); - useRetval = oldUseRetval; commaExpr->set_arg2( maybeMutate( commaExpr->get_arg2(), *this ) ); return commaExpr; @@ -733,8 +745,5 @@ Expression *Pass1::mutate( ConditionalExpr *condExpr ) { - bool oldUseRetval = useRetval; - useRetval = false; condExpr->set_arg1( maybeMutate( condExpr->get_arg1(), *this ) ); - useRetval = oldUseRetval; condExpr->set_arg2( maybeMutate( condExpr->get_arg2(), *this ) ); condExpr->set_arg3( maybeMutate( condExpr->get_arg3(), *this ) ); @@ -783,5 +792,6 @@ } else { // xxx - should this be an assertion? - throw SemanticError( "unbound type variable: " + tyParm->first + " in application ", appExpr ); + std::string x = env ? toString( *env ) : "missing env"; + throw SemanticError( x + "\n" + "unbound type variable: " + tyParm->first + " in application ", appExpr ); } // if } // if @@ -819,13 +829,20 @@ Expression *Pass1::addRetParam( ApplicationExpr *appExpr, FunctionType *function, Type *retType, std::list< Expression *>::iterator &arg ) { // Create temporary to hold return value of polymorphic function and produce that temporary as a result - // using a comma expression. Possibly change comma expression into statement expression "{}" for multiple - // return values. + // using a comma expression. assert( retType ); - ObjectDecl *newObj = makeTemporary( retType->clone() ); - Expression *paramExpr = new VariableExpr( newObj ); + + Expression * paramExpr = nullptr; + // try to use existing return value parameter if it exists, otherwise create a new temporary + if ( retVals.count( appExpr ) ) { + paramExpr = retVals[appExpr]->clone(); + } else { + ObjectDecl *newObj = makeTemporary( retType->clone() ); + paramExpr = new VariableExpr( newObj ); + } + Expression * retExpr = paramExpr->clone(); // If the type of the temporary is not polymorphic, box temporary by taking its address; // otherwise the temporary is already boxed and can be used directly. - if ( ! isPolyType( newObj->get_type(), scopeTyVars, env ) ) { + if ( ! isPolyType( paramExpr->get_result(), scopeTyVars, env ) ) { paramExpr = new AddressExpr( paramExpr ); } // if @@ -833,5 +850,5 @@ arg++; // Build a comma expression to call the function and emulate a normal return. - CommaExpr *commaExpr = new CommaExpr( appExpr, new VariableExpr( newObj ) ); + CommaExpr *commaExpr = new CommaExpr( appExpr, retExpr ); commaExpr->set_env( appExpr->get_env() ); appExpr->set_env( 0 ); @@ -851,5 +868,7 @@ Type *concrete = env->lookup( typeInst->get_name() ); if ( concrete == 0 ) { - throw SemanticError( "Unbound type variable " + typeInst->get_name() + " in ", appExpr ); + // xxx - should this be an assertion? + std::string x = env ? toString( *env ) : "missing env"; + throw SemanticError( x + "\n" + "Unbound type variable " + typeInst->get_name() + " in ", appExpr ); } // if return concrete; @@ -1259,9 +1278,6 @@ // } // std::cerr << "\n"; - bool oldUseRetval = useRetval; - useRetval = false; appExpr->get_function()->acceptMutator( *this ); mutateAll( appExpr->get_args(), *this ); - useRetval = oldUseRetval; assert( appExpr->get_function()->has_result() ); @@ -1545,6 +1561,30 @@ } + /// determines if `pref` is a prefix of `str` + bool isPrefix( const std::string & str, const std::string & pref ) { + if ( pref.size() > str.size() ) return false; + auto its = std::mismatch( pref.begin(), pref.end(), str.begin() ); + return its.first == pref.end(); + } + DeclarationWithType * Pass2::mutate( FunctionDecl *functionDecl ) { - return handleDecl( functionDecl, functionDecl->get_functionType() ); + if ( ! LinkageSpec::isBuiltin( functionDecl->get_linkage() ) ) { + // std::cerr << "mutating function: " << functionDecl->get_name() << std::endl; + } + functionDecl = safe_dynamic_cast< FunctionDecl * > ( handleDecl( functionDecl, functionDecl->get_functionType() ) ); + FunctionType * ftype = functionDecl->get_functionType(); + if ( ! ftype->get_returnVals().empty() && functionDecl->get_statements() ) { + if ( functionDecl->get_name() != "?=?" && ! isPrefix( functionDecl->get_name(), "_thunk" ) ) { // xxx - remove check for ?=? once reference types are in; remove check for prefix once thunks properly use ctor/dtors + assert( ftype->get_returnVals().size() == 1 ); + DeclarationWithType * retval = ftype->get_returnVals().front(); + if ( retval->get_name() == "" ) { + retval->set_name( "_retval" ); + } + functionDecl->get_statements()->get_kids().push_front( new DeclStmt( noLabels, retval ) ); + DeclarationWithType * newRet = retval->clone(); // for ownership purposes + ftype->get_returnVals().front() = newRet; + } + } + return functionDecl; }