From 768e03a47415f9c5aef0263175612a43e45b788e Mon Sep 17 00:00:00 2001 From: acqn Date: Sun, 19 Jul 2020 13:47:48 +0800 Subject: [PATCH] Small fixes and tidy-up based on PR review. Renamed GetReplacementType() to GetStructReplacementType(). Clarified in comments that most *Struct* facilities work for unions as well. Made it clear in some error messages with regards to structs/unions. --- src/cc65/assignment.c | 17 +++++------ src/cc65/datatype.c | 2 +- src/cc65/datatype.h | 4 +-- src/cc65/expr.c | 66 +++++++++++++++++++++++++------------------ src/cc65/function.c | 4 +-- src/cc65/stmt.c | 2 +- src/cc65/symtab.c | 10 +++---- src/cc65/symtab.h | 2 +- 8 files changed, 60 insertions(+), 47 deletions(-) diff --git a/src/cc65/assignment.c b/src/cc65/assignment.c index 2a278f4a5..ed374ef8a 100644 --- a/src/cc65/assignment.c +++ b/src/cc65/assignment.c @@ -55,15 +55,15 @@ static int CopyStruct (ExprDesc* LExpr, ExprDesc* RExpr) -/* Copy the struct represented by RExpr to the one represented by LExpr */ +/* Copy the struct/union represented by RExpr to the one represented by LExpr */ { /* If the size is that of a basic type (char, int, long), we will copy ** the struct using the primary register, otherwise we use memcpy. In ** the former case, push the address only if really needed. */ const Type* ltype = LExpr->Type; - const Type* stype = GetReplacementType (ltype); - int UseReg = (stype != LExpr->Type); + const Type* stype = GetStructReplacementType (ltype); + int UseReg = (stype != ltype); if (UseReg) { PushAddr (LExpr); @@ -105,7 +105,7 @@ static int CopyStruct (ExprDesc* LExpr, ExprDesc* RExpr) /* Push the address (or whatever is in ax in case of errors) */ g_push (CF_PTR | CF_UNSIGNED, 0); - /* Load the size of the struct into the primary */ + /* Load the size of the struct or union into the primary */ g_getimmed (CF_INT | CF_UNSIGNED | CF_CONST, CheckedSizeOf (ltype), 0); /* Call the memcpy function */ @@ -137,12 +137,13 @@ void Assignment (ExprDesc* Expr) /* Skip the '=' token */ NextToken (); - /* cc65 does not have full support for handling structs by value. Since - ** assigning structs is one of the more useful operations from this - ** family, allow it here. + /* cc65 does not have full support for handling structs or unions. Since + ** assigning structs is one of the more useful operations from this family, + ** allow it here. + ** Note: IsClassStruct() is also true for union types. */ if (IsClassStruct (ltype)) { - /* Copy the struct by value */ + /* Copy the struct or union by value */ CopyStruct (Expr, &Expr2); } else if (ED_IsBitField (Expr)) { diff --git a/src/cc65/datatype.c b/src/cc65/datatype.c index 5fcfa7115..2a989e9f8 100644 --- a/src/cc65/datatype.c +++ b/src/cc65/datatype.c @@ -225,7 +225,7 @@ Type* GetImplicitFuncType (void) -const Type* GetReplacementType (const Type* SType) +const Type* GetStructReplacementType (const Type* SType) /* Get a replacement type for passing a struct/union in the primary register */ { const Type* NewType; diff --git a/src/cc65/datatype.h b/src/cc65/datatype.h index 62ea8d06d..3464e8a16 100644 --- a/src/cc65/datatype.h +++ b/src/cc65/datatype.h @@ -243,7 +243,7 @@ Type* GetCharArrayType (unsigned Len); Type* GetImplicitFuncType (void); /* Return a type string for an inplicitly declared function */ -const Type* GetReplacementType (const Type* SType); +const Type* GetStructReplacementType (const Type* SType); /* Get a replacement type for passing a struct/union in the primary register */ Type* PointerTo (const Type* T); @@ -483,7 +483,7 @@ INLINE int IsClassPtr (const Type* T) #if defined(HAVE_INLINE) INLINE int IsClassStruct (const Type* T) -/* Return true if this is a struct type */ +/* Return true if this is a struct or union type */ { return (GetClass (T) == T_CLASS_STRUCT); } diff --git a/src/cc65/expr.c b/src/cc65/expr.c index 44137a773..828e0fd75 100644 --- a/src/cc65/expr.c +++ b/src/cc65/expr.c @@ -398,7 +398,7 @@ static unsigned FunctionParamList (FuncDesc* Func, int IsFastcall) /* Handle struct/union specially */ if (IsTypeStruct (Expr.Type) || IsTypeUnion (Expr.Type)) { /* Use the replacement type */ - Flags |= TypeOf (GetReplacementType (Expr.Type)); + Flags |= TypeOf (GetStructReplacementType (Expr.Type)); } else { /* Use the type of the argument for the push */ Flags |= TypeOf (Expr.Type); @@ -660,7 +660,7 @@ static void FunctionCall (ExprDesc* Expr) /* Handle struct/union specially */ if (IsTypeStruct (ReturnType) || IsTypeUnion (ReturnType)) { /* If there is no replacement type, then it is just the address */ - if (ReturnType == GetReplacementType (ReturnType)) { + if (ReturnType == GetStructReplacementType (ReturnType)) { /* Dereference it */ ED_IndExpr (Expr); ED_MarkExprAsRVal (Expr); @@ -1198,7 +1198,7 @@ static void ArrayRef (ExprDesc* Expr) static void StructRef (ExprDesc* Expr) -/* Process struct field after . or ->. */ +/* Process struct/union field after . or ->. */ { ident Ident; SymEntry* Field; @@ -1214,38 +1214,41 @@ static void StructRef (ExprDesc* Expr) return; } - /* Get the symbol table entry and check for a struct field */ + /* Get the symbol table entry and check for a struct/union field */ strcpy (Ident, CurTok.Ident); NextToken (); Field = FindStructField (Expr->Type, Ident); if (Field == 0) { - Error ("Struct/union has no field named '%s'", Ident); + Error ("No field named '%s' found in %s", GetBasicTypeName (Expr->Type), Ident); /* Make the expression an integer at address zero */ ED_MakeConstAbs (Expr, 0, type_int); return; } - /* A struct is usually an lvalue. If not, it is a struct passed in the - ** primary register, which is usually a result returned from a function. - ** However, it is possible that this rvalue is a result of certain - ** operations on an lvalue, and there are no reasons to disallow that. - ** So we just rely on the check on function returns to catch the errors - ** and dereference the rvalue address of the struct here. + /* A struct/union is usually an lvalue. If not, it is a struct/union passed + ** in the primary register, which is usually the result returned from a + ** function. However, it is possible that this rvalue is the result of + ** certain kind of operations on an lvalue such as assignment, and there + ** are no reasons to disallow such use cases. So we just rely on the check + ** upon function returns to catch the unsupported cases and dereference the + ** rvalue address of the struct/union here all the time. */ if (IsTypePtr (Expr->Type) || (ED_IsRVal (Expr) && ED_IsLocPrimary (Expr) && - Expr->Type == GetReplacementType (Expr->Type))) { + Expr->Type == GetStructReplacementType (Expr->Type))) { if (!ED_IsConst (Expr) && !ED_IsLocPrimary (Expr)) { - /* If we have a non-const struct pointer, load its content now */ + /* If we have a non-const struct/union pointer that is not in the + ** primary yet, load its content now. + */ LoadExpr (CF_NONE, Expr); /* Clear the offset */ Expr->IVal = 0; } - /* Dereference the expression */ + /* Dereference the address expression */ ED_IndExpr (Expr); } else if (!ED_IsLocQuasiConst (Expr) && !ED_IsLocPrimaryOrExpr (Expr)) { @@ -1255,7 +1258,7 @@ static void StructRef (ExprDesc* Expr) LoadExpr (CF_NONE, Expr); } - /* The type is the type of the field plus any qualifiers from the struct */ + /* The type is the field type plus any qualifiers from the struct/union */ if (IsClassStruct (Expr->Type)) { Q = GetQualifier (Expr->Type); } else { @@ -1280,13 +1283,22 @@ static void StructRef (ExprDesc* Expr) /* Safety check */ CHECK (Field->V.Offs + FieldSize <= StructSize); - /* The type of the operation depends on the type of the struct */ + /* The type of the operation depends on the type of the struct/union */ switch (StructSize) { - case 1: Flags = CF_CHAR | CF_UNSIGNED | CF_CONST; break; - case 2: Flags = CF_INT | CF_UNSIGNED | CF_CONST; break; - case 3: /* FALLTHROUGH */ - case 4: Flags = CF_LONG | CF_UNSIGNED | CF_CONST; break; - default: Internal ("Invalid struct size: %u", StructSize); break; + case 1: + Flags = CF_CHAR | CF_UNSIGNED | CF_CONST; + break; + case 2: + Flags = CF_INT | CF_UNSIGNED | CF_CONST; + break; + case 3: + /* FALLTHROUGH */ + case 4: + Flags = CF_LONG | CF_UNSIGNED | CF_CONST; + break; + default: + Internal ("Invalid %s size: %u", GetBasicTypeName (Expr->Type), StructSize); + break; } /* Generate a shift to get the field in the proper position in the @@ -1312,16 +1324,16 @@ static void StructRef (ExprDesc* Expr) } else { - /* Set the struct field offset */ + /* Set the struct/union field offset */ Expr->IVal += Field->V.Offs; /* Use the new type */ Expr->Type = FinalType; - /* An struct member is actually a variable. So the rules for variables - ** with respect to the reference type apply: If it's an array, it is + /* The usual rules for variables with respect to the reference types + ** apply to struct/union fields as well: If a field is an array, it is ** virtually an rvalue address, otherwise it's an lvalue reference. (A - ** function would also be an rvalue address, but a struct field cannot + ** function would also be an rvalue address, but a struct/union cannot ** contain functions). */ if (IsTypeArray (Expr->Type)) { @@ -1376,7 +1388,7 @@ static void hie11 (ExprDesc *Expr) case TOK_DOT: if (!IsClassStruct (Expr->Type)) { - Error ("Struct expected"); + Error ("Struct or union expected"); } StructRef (Expr); break; @@ -1387,7 +1399,7 @@ static void hie11 (ExprDesc *Expr) Expr->Type = ArrayToPtr (Expr->Type); } if (!IsClassPtr (Expr->Type) || !IsClassStruct (Indirect (Expr->Type))) { - Error ("Struct pointer expected"); + Error ("Struct pointer or union pointer expected"); } StructRef (Expr); break; diff --git a/src/cc65/function.c b/src/cc65/function.c index df667e8fa..4cd5565c4 100644 --- a/src/cc65/function.c +++ b/src/cc65/function.c @@ -478,7 +478,7 @@ void NewFunc (SymEntry* Func) } else { /* Handle struct/union specially */ if (IsTypeStruct (D->LastParam->Type) || IsTypeUnion (D->LastParam->Type)) { - Flags = TypeOf (GetReplacementType (D->LastParam->Type)) | CF_FORCECHAR; + Flags = TypeOf (GetStructReplacementType (D->LastParam->Type)) | CF_FORCECHAR; } else { Flags = TypeOf (D->LastParam->Type) | CF_FORCECHAR; } @@ -507,7 +507,7 @@ void NewFunc (SymEntry* Func) /* Check if we need copy for struct/union type */ RType = Param->Type; if (IsTypeStruct (RType) || IsTypeUnion (RType)) { - RType = GetReplacementType (RType); + RType = GetStructReplacementType (RType); /* If there is no replacement type, then it is just the address. ** We don't currently support this case. diff --git a/src/cc65/stmt.c b/src/cc65/stmt.c index a1384b0ed..6c839c420 100644 --- a/src/cc65/stmt.c +++ b/src/cc65/stmt.c @@ -331,7 +331,7 @@ static void ReturnStatement (void) /* Load the value into the primary */ if (IsTypeStruct (Expr.Type) || IsTypeUnion (Expr.Type)) { /* Handle struct/union specially */ - ReturnType = GetReplacementType (Expr.Type); + ReturnType = GetStructReplacementType (Expr.Type); if (ReturnType == Expr.Type) { Error ("Returning %s of this size by value is not supported", GetBasicTypeName (Expr.Type)); } diff --git a/src/cc65/symtab.c b/src/cc65/symtab.c index f19a251bd..f363d9134 100644 --- a/src/cc65/symtab.c +++ b/src/cc65/symtab.c @@ -488,24 +488,24 @@ SymEntry* FindTagSym (const char* Name) SymEntry* FindStructField (const Type* T, const char* Name) -/* Find a struct field in the fields list */ +/* Find a struct/union field in the fields list */ { SymEntry* Field = 0; - /* The given type may actually be a pointer to struct */ + /* The given type may actually be a pointer to struct/union */ if (IsTypePtr (T)) { ++T; } - /* Non-structs do not have any struct fields... */ + /* Only structs/unions have struct/union fields... */ if (IsClassStruct (T)) { /* Get a pointer to the struct/union type */ const SymEntry* Struct = GetSymEntry (T); CHECK (Struct != 0); - /* Now search in the struct symbol table. Beware: The table may not - ** exist. + /* Now search in the struct/union symbol table. Beware: The table may + ** not exist. */ if (Struct->V.S.SymTab) { Field = FindSymInTable (Struct->V.S.SymTab, Name, HashStr (Name)); diff --git a/src/cc65/symtab.h b/src/cc65/symtab.h index 62e731042..94e5f2d9b 100644 --- a/src/cc65/symtab.h +++ b/src/cc65/symtab.h @@ -134,7 +134,7 @@ SymEntry* FindTagSym (const char* Name); /* Find the symbol with the given name in the tag table */ SymEntry* FindStructField (const Type* TypeArray, const char* Name); -/* Find a struct field in the fields list */ +/* Find a struct/union field in the fields list */ unsigned short FindSPAdjustment (const char* Name); /* Search for an entry in the table of SP adjustments */