Changeset 81e768d for src

Timestamp:

Nov 28, 2024, 4:34:08 PM (11 months ago)

Author:

Michael Brooks <mlbrooks@…>

Branches:

master

Children:

46c4dea

Parents:

f5e37a4

Message:

Fix #276; add support for c-array parameters using dependent lengths.

Without this fix, declarations like

void f( int m, int n, float[m][n] );

would either

• generate bad C code, with unmangled variable names appearing in the function definition, or
• refuse to resolve a valid-c call of such a function.

tests/array-collections/c-dependent: add direct tests of such cases
tests/tuplearray: activate and expand cases which were blocked on #276
tests/array: activate case fm5y, which was blocked on #276; [noise] adjust source line numbers in .expect
tests/typedefRedef: expand coverage of "error, an array detail is different" cases; [noise] adjust source line numbers in .expect
tests/functions: [noise] adjust .expect to have resolved array sizes (extra casts) in the diffed code dump

The fix is:

• (ResolvExpr/ResolveTypeof, ResolvExpr/Resolver) Resolve the dimension expressions, where they were missed.
• (ResolvExpr/Resolver) Prevent dimension expressions that are bound to other parameters from escaping in the function's type, to where they are out of scope. In the f example above, redact the type shown to callers from void (*)(int, int, float[m][n]) to void (*)(int, int, float[][*]).
• (ResolvExpr/Unify) Relax the matching rules for such a type, when used at a call site, letting the patameters wildcard type match with the concrete type in scope at the caller's side.
• (Validate/ReplaceTypedef) Apply the former, stricter matching rules to the one place where they are still needed: detecting inconsistent typedefs.

Location:

src

Files:

• ResolvExpr/ResolveTypeof.cpp (modified) (2 diffs)
• ResolvExpr/Resolver.cpp (modified) (4 diffs)
• ResolvExpr/Unify.cpp (modified) (1 diff)
• Validate/ReplaceTypedef.cpp (modified) (4 diffs)

src/ResolvExpr/ResolveTypeof.cpp

@@ -88 +88 @@
         FixArrayDimension(const ResolveContext & context) : context( context ) {}
 
-        const ast::ArrayType * previsit (const ast::ArrayType * arrayType) {
-                if (!arrayType->dimension) return arrayType;
-                auto mutType = mutate(arrayType);
+        template< typename PtrType >
+        const PtrType * previsitImpl( const PtrType * type ) {
+                // Note: resolving dimension expressions seems to require duplicate logic,
+                // here and Resolver.cpp: handlePtrType
+
+                if (!type->dimension) return type;
+                auto mutType = mutate(type);
                 auto globalSizeType = context.global.sizeType;
                 ast::ptr<ast::Type> sizetype = globalSizeType ? globalSizeType : new ast::BasicType( ast::BasicKind::LongUnsignedInt );
-                mutType->dimension = findSingleExpression(arrayType->dimension, sizetype, context );
+                mutType->dimension = findSingleExpression(type->dimension, sizetype, context );
 
                 if (InitTweak::isConstExpr(mutType->dimension)) {
@@ -102 +106 @@
                 }
                 return mutType;
+        }
+
+        const ast::ArrayType * previsit (const ast::ArrayType * arrayType) {
+                return previsitImpl( arrayType );
+        }
+
+        const ast::PointerType * previsit (const ast::PointerType * pointerType) {
+                return previsitImpl( pointerType );
         }
 };

src/ResolvExpr/Resolver.cpp

@@ -494 +494 @@
 }
 
+// Returns a version of `ty`, with some detail redacted.
+// `ty` is that of a parameter or return of `functionDecl`.
+// Redaction:
+//   - concerns the dimension expression, when `ty` is a pointer or array
+//   - prevents escape of variables bound by other parameter declarations
+//   - replaces the whole dimension with `*` if it uses such a variable
+//   - produces the caller's view of `functionDecl`, where `ty` is from the callee/body's view
+// Example 1
+//   functionDecl:     void   f( int n, float a[][5][n + 1] );
+//   outcome:      f : void (*)( int  , float  [][5][*]     ), redaction on deepest ArrayType
+// Example 2
+//   functionDecl:     void   f( int n, float a[n] );
+//   outcome:      f : void (*)( int  , float  [*]     ), redaction on PointerType
+// Example 3
+//   in scope:         int n;
+//   functionDecl:     void   f( float a[][n] );
+//   outcome:      f : void (*)( float  [][n] ), no redaction
+static const ast::Type * redactBoundDimExprs(
+        const ast::Type * ty,
+        const ast::FunctionDecl * functionDecl
+);
+struct UsesParams {
+        const ast::FunctionDecl * functionDecl;
+        UsesParams( const ast::FunctionDecl * functionDecl ) : functionDecl(functionDecl) {}
+        bool result = false;
+        void postvisit( const ast::VariableExpr * e ) {
+                for ( auto p : functionDecl->params ) {
+                        if ( p.get() == e->var ) result = true;
+                }
+        }
+};
+struct Redactor {
+        const ast::FunctionDecl * functionDecl;
+        Redactor( const ast::FunctionDecl * functionDecl ) : functionDecl(functionDecl) {}
+        template< typename PtrType >
+        const PtrType * postvisitImpl( const PtrType * type ) {
+                if ( type->dimension && ast::Pass<UsesParams>::read( type->dimension.get(), functionDecl ) ) {
+                        // PtrType * newtype = ast::shallowCopy( type );
+                        // newtype->dimension = nullptr;
+                        // type = newtype;
+                        auto mutType = mutate(type);
+                        mutType->dimension = nullptr;
+                        type = mutType;
+                }
+                return type;
+        }
+
+        const ast::ArrayType * postvisit (const ast::ArrayType * arrayType) {
+                return postvisitImpl( arrayType );
+        }
+
+        const ast::PointerType * postvisit (const ast::PointerType * pointerType) {
+                return postvisitImpl( pointerType );
+        }
+};
+static const ast::Type * redactBoundDimExprs(
+        const ast::Type * ty,
+        const ast::FunctionDecl * functionDecl
+) {
+        if ( ast::Pass<UsesParams>::read( ty, functionDecl ) ) {
+                ast::Type * newty = ast::deepCopy( ty );
+                ast::Pass<Redactor> visitor(functionDecl);
+                ty = newty->accept(visitor);
+        }
+        return ty;
+}
+
 const ast::FunctionDecl * Resolver::previsit( const ast::FunctionDecl * functionDecl ) {
         GuardValue( functionReturn );
@@ -534 +601 @@
                         param = fixObjectType(param.strict_as<ast::ObjectDecl>(), context);
                         symtab.addId(param);
-                        paramTypes.emplace_back(param->get_type());
+                        auto exportParamT = redactBoundDimExprs( param->get_type(), mutDecl );
+                        paramTypes.emplace_back( exportParamT );
                 }
                 for (auto & ret : mutDecl->returns) {
                         ret = fixObjectType(ret.strict_as<ast::ObjectDecl>(), context);
-                        returnTypes.emplace_back(ret->get_type());
+                        auto exportRetT = redactBoundDimExprs( ret->get_type(), mutDecl );
+                        returnTypes.emplace_back( exportRetT );
                 }
                 // since function type in decl is just a view of param types, need to update that as well
@@ -699 +768 @@
 template< typename PtrType >
 const PtrType * handlePtrType( const PtrType * type, const ResolveContext & context ) {
+        // Note: resolving dimension expressions seems to require duplicate logic,
+        // here and ResolveTypeof.cpp:fixArrayType.
         if ( type->dimension ) {
                 const ast::Type * sizeType = context.global.sizeType.get();
@@ -704 +775 @@
                 assertf(dimension->env->empty(), "array dimension expr has nonempty env");
                 dimension.get_and_mutate()->env = nullptr;
-                ast::mutate_field( type, &PtrType::dimension, dimension );
+                type = ast::mutate_field( type, &PtrType::dimension, dimension );
         }
         return type;

src/ResolvExpr/Unify.cpp

@@ -292 +292 @@
                 if ( !array2 ) return;
 
-                if ( array->isVarLen != array2->isVarLen ) return;
-                if ( (array->dimension != nullptr) != (array2->dimension != nullptr) ) return;
-
-                if ( array->dimension ) {
+                // Permit cases where one side has a dimension or isVarLen,
+                // while the other side is the opposite.
+                // Acheves a wildcard-iterpretation semantics, where lack of
+                // dimension (`float a[]` or `float a[25][*]`) means
+                // "anything here is fine."
+                // Sole known case where a verbatim-match semantics is intended
+                // is typedef redefinition, for which extra checking is added
+                // in src/Validate/ReplaceTypedef.cpp.
+
+                if ( array->dimension && array2->dimension ) {
                         assert( array2->dimension );
                         // type unification calls expression unification (mutual recursion)

src/Validate/ReplaceTypedef.cpp

@@ -120 +120 @@
         }
 }
-
 struct VarLenChecker : public ast::WithShortCircuiting {
         bool result = false;
@@ -126 +125 @@
         void previsit( ast::ArrayType const * at ) { result |= at->isVarLen; }
 };
-
+static bool hasVarLen( const ast::Type * t ) {
+        return ast::Pass<VarLenChecker>::read( t );
+}
+struct ArrayTypeExtractor {
+        std::vector<const ast::ArrayType *> result;
+        void postvisit( const ast::ArrayType * at ) {
+                result.push_back( at );
+        }
+};
+static bool dimensionPresenceMismatched( const ast::Type * t0, const ast::Type * t1) {
+        std::vector<const ast::ArrayType *> at0s = std::move(
+                ast::Pass<ArrayTypeExtractor>::read( t0 ) );
+        std::vector<const ast::ArrayType *> at1s = std::move(
+                ast::Pass<ArrayTypeExtractor>::read( t1 ) );
+        assert( at0s.size() == at1s.size() );
+        for (size_t i = 0; i < at0s.size(); i++) {
+                const ast::ArrayType * at0 = at0s[i];
+                const ast::ArrayType * at1 = at1s[i];
+                assert( ResolvExpr::typesCompatible( at0, at1 ) );
+                if ( (at0->dimension != nullptr) != (at1->dimension != nullptr) ) return true;
+        }
+        return false;
+}
 ast::Decl const * ReplaceTypedefCore::postvisit(
                 ast::TypedefDecl const * decl ) {
@@ -133 +154 @@
                 ast::Type const * t0 = decl->base;
                 ast::Type const * t1 = typedefNames[ decl->name ].first->base;
+                // [hasVarLen]
                 // Cannot redefine VLA typedefs. Note: this is slightly incorrect,
                 // because our notion of VLAs at this point in the translator is
@@ -139 +161 @@
                 // constant/enumerator. The effort required to fix this corner case
                 // likely outweighs the utility of allowing it.
+                // [dimensionPresenceMismatched]
+                // Core typesCompatible logic interprets absent dimensions as wildcards,
+                // i.e. float[][*] matches float[][42].
+                // For detecting incompatible typedefs, we have to interpret them verbatim,
+                // i.e. float[] is different than float[42].
+                // But typesCompatible does assure that the pair of types is structurally
+                // consistent, outside of the dimension expressions.  This assurance guards
+                // the dimension-presence traversal.  So this traversal logic can (and does)
+                // assume that ArrayTypes will be encountered in analogous places.
                 if ( !ResolvExpr::typesCompatible( t0, t1 )
-                                || ast::Pass<VarLenChecker>::read( t0 )
-                                || ast::Pass<VarLenChecker>::read( t1 ) ) {
+                                || hasVarLen( t0 )
+                                || hasVarLen( t1 )
+                                || dimensionPresenceMismatched( t0, t1 ) ) {
                         SemanticError( decl->location, "Cannot redefine typedef %s", decl->name.c_str() );
                 }