Changeset cac8a6e for src/ResolvExpr

Timestamp:

May 22, 2018, 11:08:22 AM (8 years ago)

Author:

Thierry Delisle <tdelisle@…>

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, with_gc

Children:

639991a

Parents:

4a333d35 (diff), 2f0a0678 (diff)
Note: this is a merge changeset, the changes displayed below correspond to the merge itself.
Use the (diff) links above to see all the changes relative to each parent.

Message:

Merge branch 'master' of plg.uwaterloo.ca:software/cfa/cfa-cc

Location:

src/ResolvExpr

Files:

• AlternativeFinder.cc (modified) (3 diffs)
• Resolver.cc (modified) (4 diffs)
• TypeEnvironment.h (modified) (1 diff)
• Unify.cc (modified) (1 diff)

src/ResolvExpr/AlternativeFinder.cc

@@ -1285 +1285 @@
                 AlternativeFinder finder( indexer, env );
                 finder.targetType = toType;
-                finder.findWithAdjustment( castExpr->get_arg() );
+                finder.findWithAdjustment( castExpr->arg );
 
                 AltList candidates;
@@ -1296 +1296 @@
                         // that are cast directly.  The candidate is invalid if it has fewer results than there are types to cast
                         // to.
-                        int discardedValues = alt.expr->get_result()->size() - castExpr->get_result()->size();
+                        int discardedValues = alt.expr->result->size() - castExpr->result->size();
                         if ( discardedValues < 0 ) continue;
                         // xxx - may need to go into tuple types and extract relevant types and use unifyList. Note that currently, this does not
                         // allow casting a tuple to an atomic type (e.g. (int)([1, 2, 3]))
                         // unification run for side-effects
-                        unify( castExpr->get_result(), alt.expr->get_result(), alt.env, needAssertions,
+                        unify( castExpr->result, alt.expr->result, alt.env, needAssertions,
                                 haveAssertions, openVars, indexer );
-                        Cost thisCost = castCost( alt.expr->get_result(), castExpr->get_result(), indexer,
+                        Cost thisCost = castCost( alt.expr->result, castExpr->result, indexer,
                                 alt.env );
                         PRINT(
@@ -1726 +1726 @@
                                 // allow casting a tuple to an atomic type (e.g. (int)([1, 2, 3]))
                                 // unification run for side-effects
-                                unify( toType, alt.expr->get_result(), newEnv, needAssertions, haveAssertions, openVars, indexer ); // xxx - do some inspecting on this line... why isn't result bound to initAlt.type??
+                                unify( toType, alt.expr->result, newEnv, needAssertions, haveAssertions, openVars, indexer ); // xxx - do some inspecting on this line... why isn't result bound to initAlt.type??
 
                                 Cost thisCost = castCost( alt.expr->get_result(), toType, indexer, newEnv );

src/ResolvExpr/Resolver.cc

@@ -127 +127 @@
 
         namespace {
-                void finishExpr( Expression *expr, const TypeEnvironment &env, TypeSubstitution * oldenv = nullptr ) {
+                struct StripCasts {
+                        Expression * postmutate( CastExpr * castExpr ) {
+                                if ( castExpr->isGenerated && ResolvExpr::typesCompatible( castExpr->arg->result, castExpr->result, SymTab::Indexer() ) ) {
+                                        // generated cast is to the same type as its argument, so it's unnecessary -- remove it
+                                        Expression * expr = castExpr->arg;
+                                        castExpr->arg = nullptr;
+                                        std::swap( expr->env, castExpr->env );
+                                        return expr;
+                                }
+                                return castExpr;
+                        }
+
+                        static void strip( Expression *& expr ) {
+                                PassVisitor<StripCasts> stripper;
+                                expr = expr->acceptMutator( stripper );
+                        }
+                };
+
+                void finishExpr( Expression *&expr, const TypeEnvironment &env, TypeSubstitution * oldenv = nullptr ) {
                         expr->env = oldenv ? oldenv->clone() : new TypeSubstitution;
-                        env.makeSubstitution( *expr->get_env() );
+                        env.makeSubstitution( *expr->env );
+                        StripCasts::strip( expr ); // remove unnecessary casts that may be buried in an expression
                 }
 
@@ -414 +433 @@
 
         void Resolver::previsit( CaseStmt *caseStmt ) {
-                if ( caseStmt->get_condition() ) {
+                if ( caseStmt->condition ) {
                         std::list< InitAlternative > initAlts = currentObject.getOptions();
                         assertf( initAlts.size() == 1, "SwitchStmt did not correctly resolve an integral expression." );
@@ -420 +439 @@
                         Expression * newExpr = new CastExpr( caseStmt->condition, initAlts.front().type->clone() );
                         findSingleExpression( newExpr, indexer );
-                        CastExpr * castExpr = strict_dynamic_cast< CastExpr * >( newExpr );
-                        caseStmt->condition = castExpr->arg;
-                        castExpr->arg = nullptr;
-                        delete castExpr;
+                        // case condition cannot have a cast in C, so it must be removed, regardless of whether it performs a conversion.
+                        // Ideally we would perform the conversion internally here.
+                        if ( CastExpr * castExpr = dynamic_cast< CastExpr * >( newExpr ) ) {
+                                newExpr = castExpr->arg;
+                                castExpr->arg = nullptr;
+                                std::swap( newExpr->env, castExpr->env );
+                                delete castExpr;
+                        }
+                        caseStmt->condition = newExpr;
                 }
         }
@@ -718 +742 @@
                 initExpr->expr = nullptr;
                 std::swap( initExpr->env, newExpr->env );
-                std::swap( initExpr->inferParams, newExpr->inferParams ) ;
+                // InitExpr may have inferParams in the case where the expression specializes a function pointer,
+                // and newExpr may already have inferParams of its own, so a simple swap is not sufficient.
+                newExpr->spliceInferParams( initExpr );
                 delete initExpr;
 

src/ResolvExpr/TypeEnvironment.h

@@ -37 +37 @@
         //
         // I've seen a TU go from 54 minutes to 1 minute 34 seconds with the addition of this comparator.
+        //
+        // Note: since this compares pointers for position, minor changes in the source file that affect
+        // memory layout can alter compilation time in unpredictable ways. For example, the placement
+        // of a line directive can reorder type pointers with respect to each other so that assertions
+        // are seen in different orders, causing a potentially different number of unification calls when
+        // resolving assertions. I've seen a TU go from 36 seconds to 27 seconds by reordering line directives
+        // alone, so it would be nice to fix this comparison so that assertions compare more consistently.
+        // I've tried to modify this to compare on mangle name instead of type as the second comparator, but
+        // this causes some assertions to never be recorded. More investigation is needed.
         struct AssertCompare {
                 bool operator()( DeclarationWithType * d1, DeclarationWithType * d2 ) const {

src/ResolvExpr/Unify.cc

@@ -324 +324 @@
                 } else if ( isopen1 ) {
                         result = bindVar( var1, type2, entry1->second, env, needAssertions, haveAssertions, openVars, widenMode, indexer );
-                } else if ( isopen2 ) {
+                } else if ( isopen2 ) { // TODO: swap widenMode values in call, since type positions are flipped?
                         result = bindVar( var2, type1, entry2->second, env, needAssertions, haveAssertions, openVars, widenMode, indexer );
                 } else {