From 319ba2a876f9960ed4936f8ca05bb7894537eb02 Mon Sep 17 00:00:00 2001 From: Evan Haas Date: Mon, 23 Oct 2023 23:55:05 -0700 Subject: [PATCH 1/3] Type: improve type equality checking 1) Arrays with same child type are compatible if at least one array is incomplete 2) Functions must both have or both not have ellipses to be compatible 3) A function with no prototype is compatible with a function with a parameter list if none of the named parameter types undergo default argument promotion and the return type is compatible --- src/Parser.zig | 9 +++- src/Type.zig | 96 ++++++++++++++++++++---------------- test/cases/invalid types.c | 17 ++++--- test/cases/redefinitions.c | 26 ++++++++-- test/cases/tentative array.c | 3 +- 5 files changed, 96 insertions(+), 55 deletions(-) diff --git a/src/Parser.zig b/src/Parser.zig index 660fbf10..44f12a2a 100644 --- a/src/Parser.zig +++ b/src/Parser.zig @@ -6342,7 +6342,14 @@ fn typesCompatible(p: *Parser) Error!Result { try p.expectClosing(l_paren, .r_paren); - const compatible = first.compatible(second, p.comp); + var first_unqual = first.canonicalize(.standard); + first_unqual.qual.@"const" = false; + first_unqual.qual.@"volatile" = false; + var second_unqual = second.canonicalize(.standard); + second_unqual.qual.@"const" = false; + second_unqual.qual.@"volatile" = false; + + const compatible = first_unqual.eql(second_unqual, p.comp, true); var res = Result{ .val = Value.int(@intFromBool(compatible)), diff --git a/src/Type.zig b/src/Type.zig index ab91a138..f844e474 100644 --- a/src/Type.zig +++ b/src/Type.zig @@ -103,6 +103,35 @@ pub const Func = struct { name: StringId, name_tok: TokenIndex, }; + + fn eql(a: *const Func, b: *const Func, a_var_args: bool, b_var_args: bool, comp: *const Compilation) bool { + // return type cannot have qualifiers + if (!a.return_type.eql(b.return_type, comp, false)) return false; + + if (a.params.len != b.params.len) { + const a_no_proto = a_var_args and a.params.len == 0 and !comp.langopts.standard.atLeast(.c2x); + const b_no_proto = b_var_args and b.params.len == 0 and !comp.langopts.standard.atLeast(.c2x); + if (a_no_proto or b_no_proto) { + const maybe_has_params = if (a_no_proto) b else a; + for (maybe_has_params.params) |param| { + if (param.ty.undergoesDefaultArgPromotion(comp)) return false; + } + return true; + } + } + if (a_var_args != b_var_args) return false; + // TODO validate this + for (a.params, b.params) |param, b_qual| { + var a_unqual = param.ty; + a_unqual.qual.@"const" = false; + a_unqual.qual.@"volatile" = false; + var b_unqual = b_qual.ty; + b_unqual.qual.@"const" = false; + b_unqual.qual.@"volatile" = false; + if (!a_unqual.eql(b_unqual, comp, true)) return false; + } + return true; + } }; pub const Array = struct { @@ -447,6 +476,22 @@ pub fn isArray(ty: Type) bool { }; } +/// Whether the type is promoted if used as a variadic argument or as an argument to a function with no prototype +fn undergoesDefaultArgPromotion(ty: Type, comp: *const Compilation) bool { + return switch (ty.specifier) { + .bool => true, + .char, .uchar, .schar => true, + .short, .ushort => true, + .@"enum" => if (comp.langopts.emulate == .clang) ty.data.@"enum".isIncomplete() else false, + .float => true, + + .typeof_type => ty.data.sub_type.undergoesDefaultArgPromotion(comp), + .typeof_expr => ty.data.expr.ty.undergoesDefaultArgPromotion(comp), + .attributed => ty.data.attributed.base.undergoesDefaultArgPromotion(comp), + else => false, + }; +} + pub fn isScalar(ty: Type) bool { return ty.isInt() or ty.isScalarNonInt(); } @@ -1194,31 +1239,6 @@ pub fn annotationAlignment(comp: *const Compilation, attrs: ?[]const Attribute) return max_requested; } -/// Checks type compatibility for __builtin_types_compatible_p -/// Returns true if the unqualified version of `a_param` and `b_param` are the same -/// Ignores top-level qualifiers (e.g. `int` and `const int` are compatible) but `int *` and `const int *` are not -/// Two types that are typedefed are considered compatible if their underlying types are compatible. -/// An enum type is not considered to be compatible with another enum type even if both are compatible with the same integer type; -/// `A[]` and `A[N]` for a type `A` and integer `N` are compatible -pub fn compatible(a_param: Type, b_param: Type, comp: *const Compilation) bool { - var a_unqual = a_param.canonicalize(.standard); - a_unqual.qual.@"const" = false; - a_unqual.qual.@"volatile" = false; - var b_unqual = b_param.canonicalize(.standard); - b_unqual.qual.@"const" = false; - b_unqual.qual.@"volatile" = false; - - if (a_unqual.eql(b_unqual, comp, true)) return true; - if (!a_unqual.isArray() or !b_unqual.isArray()) return false; - - if (a_unqual.arrayLen() == null or b_unqual.arrayLen() == null) { - // incomplete arrays are compatible with arrays of the same element type - // GCC and clang ignore cv-qualifiers on arrays - return a_unqual.elemType().compatible(b_unqual.elemType(), comp); - } - return false; -} - pub fn eql(a_param: Type, b_param: Type, comp: *const Compilation, check_qualifiers: bool) bool { const a = a_param.canonicalize(.standard); const b = b_param.canonicalize(.standard); @@ -1251,29 +1271,21 @@ pub fn eql(a_param: Type, b_param: Type, comp: *const Compilation, check_qualifi .func, .var_args_func, .old_style_func, - => { - // TODO validate this - if (a.data.func.params.len != b.data.func.params.len) return false; - // return type cannot have qualifiers - if (!a.returnType().eql(b.returnType(), comp, false)) return false; - for (a.data.func.params, b.data.func.params) |param, b_qual| { - var a_unqual = param.ty; - a_unqual.qual.@"const" = false; - a_unqual.qual.@"volatile" = false; - var b_unqual = b_qual.ty; - b_unqual.qual.@"const" = false; - b_unqual.qual.@"volatile" = false; - if (!a_unqual.eql(b_unqual, comp, check_qualifiers)) return false; - } - }, + => if (!a.data.func.eql(b.data.func, a.specifier == .var_args_func, b.specifier == .var_args_func, comp)) return false, .array, .static_array, .incomplete_array, .vector, => { - if (!std.meta.eql(a.arrayLen(), b.arrayLen())) return false; - if (!a.elemType().eql(b.elemType(), comp, check_qualifiers)) return false; + const a_len = a.arrayLen(); + const b_len = b.arrayLen(); + if (a_len == null or b_len == null) { + // At least one array is incomplete; only check child type for equality + } else if (a_len.? != b_len.?) { + return false; + } + if (!a.elemType().eql(b.elemType(), comp, false)) return false; }, .variable_len_array => if (!a.elemType().eql(b.elemType(), comp, check_qualifiers)) return false, diff --git a/test/cases/invalid types.c b/test/cases/invalid types.c index b2411c2e..f5ba1d3d 100644 --- a/test/cases/invalid types.c +++ b/test/cases/invalid types.c @@ -23,14 +23,13 @@ int bar["foo"]; int baz[] = 111111E1111111111111; int qux[] = baz; -// int g[]; -// extern int h[]; -// void foo(void) { -// int i[]; -// char j[] = "hello"; -// } +int g[]; +extern int h[]; +void foo(void) { + int i[]; + char j[] = "hello"; +} -#define TESTS_SKIPPED 7 #define EXPECTED_ERRORS \ "invalid types.c:1:6: error: cannot combine with previous 'long' specifier" \ "invalid types.c:2:14: warning: duplicate 'atomic' declaration specifier [-Wduplicate-decl-specifier]" \ @@ -45,6 +44,7 @@ int qux[] = baz; "invalid types.c:8:8: note: forward declaration of 'struct Bar'" \ "invalid types.c:9:6: error: array is too large" \ "invalid types.c:11:1: warning: plain '_Complex' requires a type specifier; assuming '_Complex double'" \ + "invalid types.c:14:6: note: previous definition is here" \ "invalid types.c:16:15: error: expected identifier or '('" \ "invalid types.c:17:17: error: array has incomplete element type 'struct bar'" \ "invalid types.c:18:15: error: expected identifier or '('" \ @@ -52,4 +52,7 @@ int qux[] = baz; "invalid types.c:22:9: error: size of array has non-integer type 'char [4]'" \ "invalid types.c:23:13: error: array initializer must be an initializer list or wide string literal" \ "invalid types.c:24:13: error: array initializer must be an initializer list or wide string literal" \ + "invalid types.c:26:5: warning: tentative array definition assumed to have one element" \ + "invalid types.c:28:6: error: redefinition of 'foo'" \ + "invalid types.c:29:9: error: variable has incomplete type 'int []'" \ diff --git a/test/cases/redefinitions.c b/test/cases/redefinitions.c index 60f66e7c..c8e2d0ff 100644 --- a/test/cases/redefinitions.c +++ b/test/cases/redefinitions.c @@ -66,9 +66,24 @@ typedef MyFloat MyInt; typedef int MyOtherInt; typedef const int MyOtherInt; -#define TESTS_SKIPPED 1 -// int f(int (*)(), double (*)[3]); -// int f(int (*)(char *), double (*)[]); +int f(int (*)(), double (*)[3]); +int f(int (*)(char *), double (*)[]); + +double maximum(int n, int m, double a[n][m]); +double maximum(int n, int m, double a[*][*]); +double maximum(int n, int m, double a[ ][*]); +double maximum(int n, int m, double a[ ][m]); + +void f3(double (* restrict a)[5]); +void f3(double a[restrict][5]); +void f3(double a[restrict 3][5]); +void f3(double a[restrict static 3][5]); + +int f4(int, ...); +int f4(int); + +int f5(int (*)(), double (*)[3]); +int f5(int (*)(char), double (*)[]); // not compatible since char undergoes default argument promotion #define EXPECTED_ERRORS "redefinitions.c:4:5: error: redefinition of 'foo' as different kind of symbol" \ "redefinitions.c:1:5: note: previous definition is here" \ @@ -105,3 +120,8 @@ typedef const int MyOtherInt; "redefinitions.c:62:13: note: previous definition is here" \ "redefinitions.c:67:19: error: typedef redefinition with different types ('const int' vs 'int')" \ "redefinitions.c:66:13: note: previous definition is here" \ + "redefinitions.c:83:5: error: redefinition of 'f4' with a different type" \ + "redefinitions.c:82:5: note: previous definition is here" \ + "redefinitions.c:86:5: error: redefinition of 'f5' with a different type" \ + "redefinitions.c:85:5: note: previous definition is here" \ + diff --git a/test/cases/tentative array.c b/test/cases/tentative array.c index e9f5a824..a6370c6c 100644 --- a/test/cases/tentative array.c +++ b/test/cases/tentative array.c @@ -9,5 +9,4 @@ int arr[3]; // TODO should make the error go away #define EXPECTED_ERRORS \ "tentative array.c:1:5: warning: tentative array definition assumed to have one element" \ "tentative array.c:3:16: error: invalid application of 'sizeof' to an incomplete type 'int []'" \ - "tentative array.c:5:5: error: redefinition of 'arr' with a different type" \ - "tentative array.c:1:5: note: previous definition is here" \ + From cb278881b610c9130447016bc39baa3c67b01f33 Mon Sep 17 00:00:00 2001 From: Evan Haas Date: Tue, 24 Oct 2023 07:53:46 -0700 Subject: [PATCH 2/3] SymbolStack: remove outdated TODO comments --- src/SymbolStack.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SymbolStack.zig b/src/SymbolStack.zig index eaa5e703..f4531774 100644 --- a/src/SymbolStack.zig +++ b/src/SymbolStack.zig @@ -197,7 +197,7 @@ pub fn defineSymbol( }, .decl => if (names[i] == name) { const prev_ty = s.syms.items(.ty)[i]; - if (!ty.eql(prev_ty, p.comp, true)) { // TODO adjusted equality check + if (!ty.eql(prev_ty, p.comp, true)) { try p.errStr(.redefinition_incompatible, tok, p.tokSlice(tok)); try p.errTok(.previous_definition, s.syms.items(.tok)[i]); } @@ -243,7 +243,7 @@ pub fn declareSymbol( }, .decl => if (names[i] == name) { const prev_ty = s.syms.items(.ty)[i]; - if (!ty.eql(prev_ty, p.comp, true)) { // TODO adjusted equality check + if (!ty.eql(prev_ty, p.comp, true)) { try p.errStr(.redefinition_incompatible, tok, p.tokSlice(tok)); try p.errTok(.previous_definition, s.syms.items(.tok)[i]); } @@ -251,7 +251,7 @@ pub fn declareSymbol( }, .def, .constexpr => if (names[i] == name) { const prev_ty = s.syms.items(.ty)[i]; - if (!ty.eql(prev_ty, p.comp, true)) { // TODO adjusted equality check + if (!ty.eql(prev_ty, p.comp, true)) { try p.errStr(.redefinition_incompatible, tok, p.tokSlice(tok)); try p.errTok(.previous_definition, s.syms.items(.tok)[i]); break; From e9f83cbb14b89ba58fc1659fb6a636c6e47e603f Mon Sep 17 00:00:00 2001 From: Evan Haas Date: Tue, 24 Oct 2023 07:53:59 -0700 Subject: [PATCH 3/3] tests: update comment text for skipped test --- test/cases/tentative array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cases/tentative array.c b/test/cases/tentative array.c index a6370c6c..95326776 100644 --- a/test/cases/tentative array.c +++ b/test/cases/tentative array.c @@ -2,7 +2,7 @@ int arr[]; _Static_assert(sizeof arr,""); -int arr[3]; // TODO should make the error go away +int arr[3]; // TODO should make the warning go away #define SKIPPED_TESTS 1