Index: src/GenPoly/Box.cc =================================================================== --- src/GenPoly/Box.cc (revision 31f379c6412cdf1bd9e7efed78d6788bbf58ed10) +++ 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; } Index: src/InitTweak/GenInit.cc =================================================================== --- src/InitTweak/GenInit.cc (revision 31f379c6412cdf1bd9e7efed78d6788bbf58ed10) +++ src/InitTweak/GenInit.cc (revision cce94296a61149881755d331f2f292f92c7f6c5c) @@ -46,5 +46,6 @@ ReturnFixer(); - using GenPoly::PolyMutator::mutate; + typedef GenPoly::PolyMutator Parent; + using Parent::mutate; virtual DeclarationWithType * mutate( FunctionDecl *functionDecl ) override; virtual Statement * mutate( ReturnStmt * returnStmt ) override; @@ -142,17 +143,15 @@ // hands off if the function returns an lvalue - we don't want to allocate a temporary if a variable's address // is being returned + // Note: under the assumption that assignments return *this, checking for ?=? here is an optimization, since it shouldn't be necessary to copy construct `this`. This is a temporary optimization until reference types are added, at which point this should be removed, along with the analogous optimization in copy constructor generation. if ( returnStmt->get_expr() && returnVals.size() == 1 && funcName != "?=?" && ! returnVals.front()->get_type()->get_isLvalue() ) { - // ensure return value is not destructed by explicitly creating - // an empty SingleInit node wherein maybeConstruct is false - ObjectDecl *newObj = new ObjectDecl( tempNamer.newName(), DeclarationNode::NoStorageClass, LinkageSpec::C, 0, returnVals.front()->get_type()->clone(), new ListInit( std::list(), noDesignators, false ) ); - stmtsToAdd.push_back( new DeclStmt( noLabels, newObj ) ); - - // and explicitly create the constructor expression separately + // explicitly construct the return value using the return expression and the retVal object + assertf( returnVals.front()->get_name() != "", "Function %s has unnamed return value\n", funcName.c_str() ); UntypedExpr *construct = new UntypedExpr( new NameExpr( "?{}" ) ); - construct->get_args().push_back( new AddressExpr( new VariableExpr( newObj ) ) ); + construct->get_args().push_back( new AddressExpr( new VariableExpr( returnVals.front() ) ) ); construct->get_args().push_back( returnStmt->get_expr() ); stmtsToAdd.push_back(new ExprStmt(noLabels, construct)); - returnStmt->set_expr( new VariableExpr( newObj ) ); + // return the retVal object + returnStmt->set_expr( new VariableExpr( returnVals.front() ) ); } // if return returnStmt; @@ -160,27 +159,10 @@ DeclarationWithType* ReturnFixer::mutate( FunctionDecl *functionDecl ) { - // xxx - need to handle named return values - this pass may need to happen - // after resolution? the ordering is tricky because return statements must be - // constructed - the simplest way to do that (while also handling multiple - // returns) is to structure the returnVals into a tuple, as done here. - // however, if the tuple return value is structured before resolution, - // it's difficult to resolve named return values, since the name is lost - // in conversion to a tuple. this might be easiest to deal with - // after reference types are added, as it may then be possible to - // uniformly move named return values to the parameter list directly ValueGuard< FunctionType * > oldFtype( ftype ); ValueGuard< std::string > oldFuncName( funcName ); ftype = functionDecl->get_functionType(); - std::list< DeclarationWithType * > & retVals = ftype->get_returnVals(); - if ( retVals.size() > 1 ) { - TupleType * tupleType = safe_dynamic_cast< TupleType * >( ResolvExpr::extractResultType( ftype ) ); - ObjectDecl * newRet = new ObjectDecl( tempNamer.newName(), DeclarationNode::NoStorageClass, LinkageSpec::C, 0, tupleType, new ListInit( std::list(), noDesignators, false ) ); - retVals.clear(); - retVals.push_back( newRet ); - } funcName = functionDecl->get_name(); - DeclarationWithType * decl = Mutator::mutate( functionDecl ); - return decl; + return Parent::mutate( functionDecl ); } Index: src/SymTab/Validate.cc =================================================================== --- src/SymTab/Validate.cc (revision 31f379c6412cdf1bd9e7efed78d6788bbf58ed10) +++ src/SymTab/Validate.cc (revision cce94296a61149881755d331f2f292f92c7f6c5c) @@ -86,4 +86,12 @@ }; + /// Fix return types so that every function returns exactly one value + class ReturnTypeFixer : public Visitor { + public: + static void fix( std::list< Declaration * > &translationUnit ); + + virtual void visit( FunctionType * ftype ); + }; + /// Replaces enum types by int, and function or array types in function parameter and return lists by appropriate pointers. class EnumAndPointerDecayPass : public Visitor { @@ -94,8 +102,8 @@ /// Associates forward declarations of aggregates with their definitions - class Pass2 final : public Indexer { + class LinkReferenceToTypes final : public Indexer { typedef Indexer Parent; public: - Pass2( bool doDebug, const Indexer *indexer ); + LinkReferenceToTypes( bool doDebug, const Indexer *indexer ); private: using Indexer::visit; @@ -193,5 +201,5 @@ void validate( std::list< Declaration * > &translationUnit, bool doDebug ) { EnumAndPointerDecayPass epc; - Pass2 pass2( doDebug, 0 ); + LinkReferenceToTypes lrt( doDebug, 0 ); Pass3 pass3( 0 ); CompoundLiteral compoundliteral; @@ -199,7 +207,8 @@ EliminateTypedef::eliminateTypedef( translationUnit ); HoistStruct::hoistStruct( translationUnit ); + ReturnTypeFixer::fix( translationUnit ); // must happen before autogen autogenerateRoutines( translationUnit ); // moved up, used to be below compoundLiteral - currently needs EnumAndPointerDecayPass acceptAll( translationUnit, epc ); - acceptAll( translationUnit, pass2 ); + acceptAll( translationUnit, lrt ); ReturnChecker::checkFunctionReturns( translationUnit ); compoundliteral.mutateDeclarationList( translationUnit ); @@ -210,8 +219,8 @@ void validateType( Type *type, const Indexer *indexer ) { EnumAndPointerDecayPass epc; - Pass2 pass2( false, indexer ); + LinkReferenceToTypes lrt( false, indexer ); Pass3 pass3( indexer ); type->accept( epc ); - type->accept( pass2 ); + type->accept( lrt ); type->accept( pass3 ); } @@ -324,5 +333,5 @@ } - Pass2::Pass2( bool doDebug, const Indexer *other_indexer ) : Indexer( doDebug ) { + LinkReferenceToTypes::LinkReferenceToTypes( bool doDebug, const Indexer *other_indexer ) : Indexer( doDebug ) { if ( other_indexer ) { indexer = other_indexer; @@ -332,5 +341,5 @@ } - void Pass2::visit( StructInstType *structInst ) { + void LinkReferenceToTypes::visit( StructInstType *structInst ) { Parent::visit( structInst ); StructDecl *st = indexer->lookupStruct( structInst->get_name() ); @@ -346,5 +355,5 @@ } - void Pass2::visit( UnionInstType *unionInst ) { + void LinkReferenceToTypes::visit( UnionInstType *unionInst ) { Parent::visit( unionInst ); UnionDecl *un = indexer->lookupUnion( unionInst->get_name() ); @@ -359,5 +368,5 @@ } - void Pass2::visit( TraitInstType *contextInst ) { + void LinkReferenceToTypes::visit( TraitInstType *contextInst ) { Parent::visit( contextInst ); if ( contextInst->get_name() == "sized" ) { @@ -398,5 +407,5 @@ } - void Pass2::visit( StructDecl *structDecl ) { + void LinkReferenceToTypes::visit( StructDecl *structDecl ) { // visit struct members first so that the types of self-referencing members are updated properly Parent::visit( structDecl ); @@ -412,5 +421,5 @@ } - void Pass2::visit( UnionDecl *unionDecl ) { + void LinkReferenceToTypes::visit( UnionDecl *unionDecl ) { Parent::visit( unionDecl ); if ( ! unionDecl->get_members().empty() ) { @@ -425,5 +434,5 @@ } - void Pass2::visit( TypeInstType *typeInst ) { + void LinkReferenceToTypes::visit( TypeInstType *typeInst ) { if ( NamedTypeDecl *namedTypeDecl = lookupType( typeInst->get_name() ) ) { if ( TypeDecl *typeDecl = dynamic_cast< TypeDecl * >( namedTypeDecl ) ) { @@ -747,4 +756,34 @@ return new VariableExpr( newtempvar ); } + + void ReturnTypeFixer::fix( std::list< Declaration * > &translationUnit ) { + ReturnTypeFixer fixer; + acceptAll( translationUnit, fixer ); + } + + void ReturnTypeFixer::visit( FunctionType * ftype ) { + static UniqueName tempNamer( "_retval" ); + + // xxx - need to handle named return values - this information needs to be saved somehow + // so that resolution has access to the names. + // Note that this pass needs to happen early so that other passes which look for tuple types + // find them in all of the right places, including function return types. + std::list< DeclarationWithType * > & retVals = ftype->get_returnVals(); + if ( retVals.size() > 1 ) { + // generate a single return parameter which is the tuple of all of the return values + TupleType * tupleType = safe_dynamic_cast< TupleType * >( ResolvExpr::extractResultType( ftype ) ); + // ensure return value is not destructed by explicitly creating an empty ListInit node wherein maybeConstruct is false. + ObjectDecl * newRet = new ObjectDecl( tempNamer.newName(), DeclarationNode::NoStorageClass, LinkageSpec::Cforall, 0, tupleType, new ListInit( std::list(), noDesignators, false ) ); + deleteAll( retVals ); + retVals.clear(); + retVals.push_back( newRet ); + } else if ( retVals.size() == 1 ) { + // ensure other return values have a name + DeclarationWithType * ret = retVals.front(); + if ( ret->get_name() == "" ) { + ret->set_name( tempNamer.newName() ); + } + } + } } // namespace SymTab