Changeset babeeda

Timestamp:

Mar 1, 2018, 2:48:18 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:

a8a2b0a

Parents:

2103a51

Message:

Implement self assignment warning

Location:

src

Files:

• Common/SemanticError.h (modified) (1 diff)
• Common/utility.h (modified) (1 diff)
• InitTweak/FixInit.cc (modified) (3 diffs)

src/Common/SemanticError.h

@@ -37 +37 @@
 
 constexpr const char * const WarningFormats[] = {
-
+        "self assignment of expression: %s",
 };
 
 enum class Warning {
+        SelfAssignment,
         NUMBER_OF_WARNINGS, //This MUST be the last warning
 };

src/Common/utility.h

@@ -162 +162 @@
 }
 
+#define toCString( ... ) toString( __VA_ARGS__ ).c_str()
+
 // replace element of list with all elements of another list
 template< typename T >

src/InitTweak/FixInit.cc

@@ -68 +68 @@
                 typedef std::unordered_map< int, int > UnqCount;
 
+                struct SelfAssignChecker {
+                        void previsit( ApplicationExpr * appExpr );
+                };
+
                 struct InsertImplicitCalls : public WithTypeSubstitution {
                         /// wrap function application expressions as ImplicitCopyCtorExpr nodes so that it is easy to identify which
@@ -235 +239 @@
 
         void fix( std::list< Declaration * > & translationUnit, const std::string & filename, bool inLibrary ) {
+                PassVisitor<SelfAssignChecker> checker;
+                acceptAll( translationUnit, checker );
+
                 // fixes ConstructorInit for global variables. should happen before fixInitializers.
                 InitTweak::fixGlobalInit( translationUnit, filename, inLibrary );
@@ -309 +316 @@
                         PassVisitor<FixCtorExprs> fixer;
                         mutateAll( translationUnit, fixer );
+                }
+
+                namespace {
+                        // Relatively simple structural comparison for expressions, needed to determine
+                        // if two expressions are "the same" (used to determine if self assignment occurs)
+                        struct StructuralChecker {
+                                Expression * stripCasts( Expression * expr ) {
+                                        // this might be too permissive. It's possible that only particular casts are relevant.
+                                        while ( CastExpr * cast = dynamic_cast< CastExpr * >( expr ) ) {
+                                                expr = cast->arg;
+                                        }
+                                        return expr;
+                                }
+
+                                void previsit( Expression * ) {
+                                        // anything else does not qualify
+                                        isSimilar = false;
+                                }
+
+                                template<typename T>
+                                T * cast( Expression * node ) {
+                                        // all expressions need to ignore casts, so this bit has been factored out
+                                        return dynamic_cast< T * >( stripCasts( node ) );
+                                }
+
+                                // ignore casts
+                                void previsit( CastExpr * ) {}
+
+                                void previsit( MemberExpr * memExpr ) {
+                                        if ( MemberExpr * otherMember = cast< MemberExpr >( other ) ) {
+                                                if ( otherMember->member == memExpr->member ) {
+                                                        other = otherMember->aggregate;
+                                                        return;
+                                                }
+                                        }
+                                        isSimilar = false;
+                                }
+
+                                void previsit( VariableExpr * varExpr ) {
+                                        if ( VariableExpr * otherVar = cast< VariableExpr >( other ) ) {
+                                                if ( otherVar->var == varExpr->var ) {
+                                                        return;
+                                                }
+                                        }
+                                        isSimilar = false;
+                                }
+
+                                void previsit( AddressExpr * ) {
+                                        if ( AddressExpr * addrExpr = cast< AddressExpr >( other ) ) {
+                                                other = addrExpr->arg;
+                                                return;
+                                        }
+                                        isSimilar = false;
+                                }
+
+                                Expression * other = nullptr;
+                                bool isSimilar = true;
+                        };
+
+                        bool structurallySimilar( Expression * e1, Expression * e2 ) {
+                                PassVisitor<StructuralChecker> checker;
+                                checker.pass.other = e2;
+                                e1->accept( checker );
+                                return checker.pass.isSimilar;
+                        }
+                }
+
+                void SelfAssignChecker::previsit( ApplicationExpr * appExpr ) {
+                        DeclarationWithType * function = getFunction( appExpr );
+                        if ( isAssignment( function ) ) {
+                                if ( appExpr->args.size() == 2 ) {
+                                        // check for structural similarity (same variable use, ignore casts, etc. - but does not look too deeply, anything looking like a function is off limits)
+                                        if ( structurallySimilar( appExpr->args.front(), appExpr->args.back() ) ) {
+                                                SemanticWarning( appExpr->location, Warning::SelfAssignment, toCString( appExpr->args.front() ) );
+                                        }
+                                }
+                        }
                 }