Changeset cce9429

Timestamp:

Dec 13, 2016, 6:42:39 PM (7 years ago)

Author:

Rob Schluntz <rschlunt@…>

Branches:

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

Children:

1aa4b71

Parents:

31f379c

Message:

fix function return type in Validate and add single return decl, construct the return decl, fix polymorphic functions to use the return decl

Location:

src

Files:

• GenPoly/Box.cc (modified) (13 diffs)
• InitTweak/GenInit.cc (modified) (3 diffs)
• SymTab/Validate.cc (modified) (13 diffs)

src/GenPoly/Box.cc

@@ -131 +131 @@
                         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 +498 @@
                 }
 
-                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 +668 @@
 
                                 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 +700 @@
                                         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 +712 @@
                                 dtorOps.endScope();
                                 retval = oldRetval;
-                                useRetval = oldUseRetval;
                                 doEndScope();
                         } // if
@@ -724 +723 @@
 
                 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 +745 @@
 
                 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 +792 @@
                                         } 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 +829 @@
                 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 +850 @@
                         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 +868 @@
                                 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 +1278 @@
                         // }
                         // 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 +1561 @@
                 }
 
+                /// 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;
                 }
 

src/InitTweak/GenInit.cc

@@ -46 +46 @@
                 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 +143 @@
                 // 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<Initializer*>(), 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 +159 @@
 
         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<Initializer*>(), noDesignators, false ) );
-                        retVals.clear();
-                        retVals.push_back( newRet );
-                }
                 funcName = functionDecl->get_name();
-                DeclarationWithType * decl = Mutator::mutate( functionDecl );
-                return decl;
+                return Parent::mutate( functionDecl );
         }
 

src/SymTab/Validate.cc

@@ -86 +86 @@
         };
 
+        /// 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 +102 @@
 
         /// 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 +201 @@
         void validate( std::list< Declaration * > &translationUnit, bool doDebug ) {
                 EnumAndPointerDecayPass epc;
-                Pass2 pass2( doDebug, 0 );
+                LinkReferenceToTypes lrt( doDebug, 0 );
                 Pass3 pass3( 0 );
                 CompoundLiteral compoundliteral;
@@ -199 +207 @@
                 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 +219 @@
         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 +333 @@
         }
 
-        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 +341 @@
         }
 
-        void Pass2::visit( StructInstType *structInst ) {
+        void LinkReferenceToTypes::visit( StructInstType *structInst ) {
                 Parent::visit( structInst );
                 StructDecl *st = indexer->lookupStruct( structInst->get_name() );
@@ -346 +355 @@
         }
 
-        void Pass2::visit( UnionInstType *unionInst ) {
+        void LinkReferenceToTypes::visit( UnionInstType *unionInst ) {
                 Parent::visit( unionInst );
                 UnionDecl *un = indexer->lookupUnion( unionInst->get_name() );
@@ -359 +368 @@
         }
 
-        void Pass2::visit( TraitInstType *contextInst ) {
+        void LinkReferenceToTypes::visit( TraitInstType *contextInst ) {
                 Parent::visit( contextInst );
                 if ( contextInst->get_name() == "sized" ) {
@@ -398 +407 @@
         }
 
-        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 +421 @@
         }
 
-        void Pass2::visit( UnionDecl *unionDecl ) {
+        void LinkReferenceToTypes::visit( UnionDecl *unionDecl ) {
                 Parent::visit( unionDecl );
                 if ( ! unionDecl->get_members().empty() ) {
@@ -425 +434 @@
         }
 
-        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 +756 @@
                 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<Initializer*>(), 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