Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type: improve type equality checking #528

Merged
merged 3 commits into from
Oct 24, 2023
Merged

Type: improve type equality checking #528

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
319ba2a
Type: improve type equality checking
ehaas Oct 24, 2023
cb27888
SymbolStack: remove outdated TODO comments
ehaas Oct 24, 2023
e9f83cb
tests: update comment text for skipped test
ehaas Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/Parser.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
96 changes: 54 additions & 42 deletions src/Type.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,

Expand Down
17 changes: 10 additions & 7 deletions test/cases/invalid types.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]" \
Expand All @@ -45,11 +44,15 @@ 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 '('" \
"invalid types.c:19:17: error: array has incomplete element type 'struct bar []'" \
"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 []'" \

26 changes: 23 additions & 3 deletions test/cases/redefinitions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" \
Expand Down Expand Up @@ -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" \

3 changes: 1 addition & 2 deletions test/cases/tentative array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" \

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also remove the TODO and SKIPPED_TESTS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this one the warning: tentative array definition assumed to have one element should not happen since the definition is later completed. I think we'd have to do something similar to what we do for tentative_defs in the Parser since we don't want to issue the warning until we're done and have determined the definition is incomplete.

Loading