Skip to content

Commit

Permalink
Merge pull request #717 from Organ1sm/implicit-integer-conversion
Browse files Browse the repository at this point in the history
Parser: Report a Warning if implicit integer conversion truncates value or sign conversion.
  • Loading branch information
Vexu authored Jul 10, 2024
2 parents 8a885d8 + a97148d commit b61abd3
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 26 deletions.
2 changes: 2 additions & 0 deletions src/aro/Diagnostics.zig
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ pub const Options = struct {
normalized: Kind = .default,
@"shift-count-negative": Kind = .default,
@"shift-count-overflow": Kind = .default,
@"constant-conversion": Kind = .default,
@"sign-conversion": Kind = .default,
};

const Diagnostics = @This();
Expand Down
12 changes: 12 additions & 0 deletions src/aro/Diagnostics/messages.def
Original file line number Diff line number Diff line change
Expand Up @@ -1658,6 +1658,18 @@ vla
.kind = .off
.opt = W("vla")

int_value_changed
.msg = "implicit conversion from {s}"
.extra = .str
.kind = .warning
.opt = W("constant-conversion")

sign_conversion
.msg = "implicit conversion changes signedness: {s}"
.extra = .str
.kind = .off
.opt = W("sign-conversion")

float_overflow_conversion
.msg = "implicit conversion of non-finite value from {s} is undefined"
.extra = .str
Expand Down
23 changes: 15 additions & 8 deletions src/aro/Parser.zig
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ pub fn typePairStrExtra(p: *Parser, a: Type, msg: []const u8, b: Type) ![]const
return try p.comp.diagnostics.arena.allocator().dupe(u8, p.strings.items[strings_top..]);
}

pub fn floatValueChangedStr(p: *Parser, res: *Result, old_value: Value, int_ty: Type) ![]const u8 {
pub fn valueChangedStr(p: *Parser, res: *Result, old_value: Value, int_ty: Type) ![]const u8 {
const strings_top = p.strings.items.len;
defer p.strings.items.len = strings_top;

Expand Down Expand Up @@ -2623,7 +2623,7 @@ fn enumSpec(p: *Parser) Error!Type {
continue;

const symbol = p.syms.getPtr(field.name, .vars);
try symbol.val.intCast(dest_ty, p.comp);
_ = try symbol.val.intCast(dest_ty, p.comp);
symbol.ty = dest_ty;
p.nodes.items(.ty)[@intFromEnum(field_nodes[i])] = dest_ty;
field.ty = dest_ty;
Expand Down Expand Up @@ -5574,7 +5574,14 @@ pub const Result = struct {
try res.implicitCast(p, .complex_float_to_complex_int);
}
} else if (!res.ty.eql(int_ty, p.comp, true)) {
try res.val.intCast(int_ty, p.comp);
const old_val = res.val;
const value_change_kind = try res.val.intCast(int_ty, p.comp);
switch (value_change_kind) {
.none => {},
.truncated => try p.errStr(.int_value_changed, tok, try p.valueChangedStr(res, old_val, int_ty)),
.sign_changed => try p.errStr(.sign_conversion, tok, try p.typePairStrExtra(res.ty, " to ", int_ty)),
}

const old_real = res.ty.isReal();
const new_real = int_ty.isReal();
if (old_real and new_real) {
Expand Down Expand Up @@ -5605,8 +5612,8 @@ pub const Result = struct {
.none => return p.errStr(.float_to_int, tok, try p.typePairStrExtra(res.ty, " to ", int_ty)),
.out_of_range => return p.errStr(.float_out_of_range, tok, try p.typePairStrExtra(res.ty, " to ", int_ty)),
.overflow => return p.errStr(.float_overflow_conversion, tok, try p.typePairStrExtra(res.ty, " to ", int_ty)),
.nonzero_to_zero => return p.errStr(.float_zero_conversion, tok, try p.floatValueChangedStr(res, old_value, int_ty)),
.value_changed => return p.errStr(.float_value_changed, tok, try p.floatValueChangedStr(res, old_value, int_ty)),
.nonzero_to_zero => return p.errStr(.float_zero_conversion, tok, try p.valueChangedStr(res, old_value, int_ty)),
.value_changed => return p.errStr(.float_value_changed, tok, try p.valueChangedStr(res, old_value, int_ty)),
}
}

Expand Down Expand Up @@ -5674,7 +5681,7 @@ pub const Result = struct {
res.ty = ptr_ty;
try res.implicitCast(p, .bool_to_pointer);
} else if (res.ty.isInt()) {
try res.val.intCast(ptr_ty, p.comp);
_ = try res.val.intCast(ptr_ty, p.comp);
res.ty = ptr_ty;
try res.implicitCast(p, .int_to_pointer);
}
Expand Down Expand Up @@ -6007,7 +6014,7 @@ pub const Result = struct {
try p.errStr(.cast_to_incomplete_type, l_paren, try p.typeStr(to));
return error.ParsingFailed;
}
try res.val.intCast(to, p.comp);
_ = try res.val.intCast(to, p.comp);
}
} else if (to.get(.@"union")) |union_ty| {
if (union_ty.data.record.hasFieldOfType(res.ty, p.comp)) {
Expand Down Expand Up @@ -8258,7 +8265,7 @@ fn charLiteral(p: *Parser) Error!Result {
// > that of the single character or escape sequence is converted to type int.
// This conversion only matters if `char` is signed and has a high-order bit of `1`
if (char_kind == .char and !is_multichar and val > 0x7F and p.comp.getCharSignedness() == .signed) {
try value.intCast(.{ .specifier = .char }, p.comp);
_ = try value.intCast(.{ .specifier = .char }, p.comp);
}

const res = Result{
Expand Down
57 changes: 42 additions & 15 deletions src/aro/Value.zig
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ pub fn floatToInt(v: *Value, dest_ty: Type, comp: *Compilation) !FloatToIntChang
};

// The float is reduced in rational.setFloat, so we assert that denominator is equal to one
const big_one = std.math.big.int.Const{ .limbs = &.{1}, .positive = true };
const big_one = BigIntConst{ .limbs = &.{1}, .positive = true };
assert(rational.q.toConst().eqlAbs(big_one));

if (is_negative) {
Expand Down Expand Up @@ -230,23 +230,50 @@ pub fn intToFloat(v: *Value, dest_ty: Type, comp: *Compilation) !void {
};
}

pub const IntCastChangeKind = enum {
/// value did not change
none,
/// Truncation occurred (e.g., i32 to i16)
truncated,
/// Sign conversion occurred (e.g., i32 to u32)
sign_changed,
};

/// Truncates or extends bits based on type.
/// `.none` value remains unchanged.
pub fn intCast(v: *Value, dest_ty: Type, comp: *Compilation) !void {
if (v.opt_ref == .none) return;
const bits: usize = @intCast(dest_ty.bitSizeof(comp).?);
pub fn intCast(v: *Value, dest_ty: Type, comp: *Compilation) !IntCastChangeKind {
if (v.opt_ref == .none) return .none;

const dest_bits: usize = @intCast(dest_ty.bitSizeof(comp).?);
const dest_signed = dest_ty.signedness(comp) == .signed;

var space: BigIntSpace = undefined;
const big = v.toBigInt(&space, comp);
const value_bits = big.bitCountTwosComp();

// if big is negative, then is signed.
const src_signed = !big.positive;
const sign_change = src_signed != dest_signed;

const limbs = try comp.gpa.alloc(
std.math.big.Limb,
std.math.big.int.calcTwosCompLimbCount(@max(big.bitCountTwosComp(), bits)),
std.math.big.int.calcTwosCompLimbCount(@max(value_bits, dest_bits)),
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
result_bigint.truncate(big, dest_ty.signedness(comp), bits);

var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };
result_bigint.truncate(big, dest_ty.signedness(comp), dest_bits);

v.* = try intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });

const truncation_occurred = value_bits > dest_bits;
if (truncation_occurred) {
return .truncated;
} else if (sign_change) {
return .sign_changed;
} else {
return .none;
}
}

/// Converts the stored value to a float of the specified type
Expand Down Expand Up @@ -509,7 +536,7 @@ pub fn add(res: *Value, lhs: Value, rhs: Value, ty: Type, comp: *Compilation) !b
std.math.big.int.calcTwosCompLimbCount(bits),
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

const overflowed = result_bigint.addWrap(lhs_bigint, rhs_bigint, ty.signedness(comp), bits);
res.* = try intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand Down Expand Up @@ -552,7 +579,7 @@ pub fn sub(res: *Value, lhs: Value, rhs: Value, ty: Type, comp: *Compilation) !b
std.math.big.int.calcTwosCompLimbCount(bits),
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

const overflowed = result_bigint.subWrap(lhs_bigint, rhs_bigint, ty.signedness(comp), bits);
res.* = try intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand Down Expand Up @@ -736,7 +763,7 @@ pub fn bitOr(lhs: Value, rhs: Value, comp: *Compilation) !Value {
@max(lhs_bigint.limbs.len, rhs_bigint.limbs.len),
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

result_bigint.bitOr(lhs_bigint, rhs_bigint);
return intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand All @@ -754,7 +781,7 @@ pub fn bitXor(lhs: Value, rhs: Value, comp: *Compilation) !Value {
@max(lhs_bigint.limbs.len, rhs_bigint.limbs.len) + extra,
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

result_bigint.bitXor(lhs_bigint, rhs_bigint);
return intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand All @@ -777,7 +804,7 @@ pub fn bitAnd(lhs: Value, rhs: Value, comp: *Compilation) !Value {

const limbs = try comp.gpa.alloc(std.math.big.Limb, limb_count);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

result_bigint.bitAnd(lhs_bigint, rhs_bigint);
return intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand All @@ -793,7 +820,7 @@ pub fn bitNot(val: Value, ty: Type, comp: *Compilation) !Value {
std.math.big.int.calcTwosCompLimbCount(bits),
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

result_bigint.bitNotWrap(val_bigint, ty.signedness(comp), bits);
return intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand All @@ -819,7 +846,7 @@ pub fn shl(res: *Value, lhs: Value, rhs: Value, ty: Type, comp: *Compilation) !b
lhs_bigint.limbs.len + (shift / (@sizeOf(std.math.big.Limb) * 8)) + 1,
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

result_bigint.shiftLeft(lhs_bigint, shift);
const signedness = ty.signedness(comp);
Expand Down Expand Up @@ -853,7 +880,7 @@ pub fn shr(lhs: Value, rhs: Value, ty: Type, comp: *Compilation) !Value {
std.math.big.int.calcTwosCompLimbCount(bits),
);
defer comp.gpa.free(limbs);
var result_bigint = std.math.big.int.Mutable{ .limbs = limbs, .positive = undefined, .len = undefined };
var result_bigint = BigIntMutable{ .limbs = limbs, .positive = undefined, .len = undefined };

result_bigint.shiftRight(lhs_bigint, shift);
return intern(comp, .{ .int = .{ .big_int = result_bigint.toConst() } });
Expand Down
14 changes: 14 additions & 0 deletions test/cases/assignment.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ void different_sign(void) {
y = &x;
}

void constant_sign_conversion(void) {
unsigned char x = 1000;

#pragma GCC diagnostic push
#pragma GCC diagnostic warning "-Wsign-conversion"
unsigned int u = -1;
#pragma GCC diagnostic pop
}


#define EXPECTED_ERRORS "assignment.c:2:7: error: expression is not assignable" \
"assignment.c:4:7: error: expression is not assignable" \
"assignment.c:6:7: warning: implicit conversion from 'float' to '_Bool' changes value from 5.5 to true [-Wfloat-conversion]" \
Expand All @@ -109,4 +119,8 @@ void different_sign(void) {
"assignment.c:72:12: error: variable has incomplete type 'enum E'" \
"assignment.c:79:7: error: expression is not assignable" \
"assignment.c:86:7: warning: incompatible pointer types assigning to 'unsigned int *' from incompatible type 'int *' converts between pointers to integer types with different sign [-Wpointer-sign]" \
"assignment.c:90:23: warning: implicit conversion from 'int' to 'unsigned char' changes value from 1000 to 232 [-Wconstant-conversion]" \
"assignment.c:94:22: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]" \



3 changes: 2 additions & 1 deletion test/cases/binary expressions.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ int bar(void) {
return 1U < 2U;
}

int baz = 0xFFFFFFFFFF + 1u;
int baz = 0xFFFFFFFFFFLL + 1u;
int qux = 1/"foo";
int quux = "foo"?1:2;

Expand Down Expand Up @@ -105,6 +105,7 @@ _Static_assert((-10 & -1) == -10, "");
"binary expressions.c:33:5: error: expected statement" \
"binary expressions.c:38:5: error: expected expression" \
"binary expressions.c:39:13: error: expected expression" \
"binary expressions.c:48:11: warning: implicit conversion from 'long long' to 'int' changes non-zero value from 1099511627776 to 0 [-Wconstant-conversion]" \
"binary expressions.c:49:12: error: invalid operands to binary expression ('int' and 'char *')" \
"binary expressions.c:55:9: error: invalid operands to binary expression ('double' and 'struct S')" \
"binary expressions.c:56:9: error: invalid operands to binary expression ('double' and 'struct S')" \
Expand Down
1 change: 1 addition & 0 deletions test/cases/constexpr.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ static_assert(!const_bool_false);
#define EXPECTED_ERRORS "constexpr.c:5:14: error: cannot combine with previous 'thread_local' specifier" \
"constexpr.c:7:19: error: invalid storage class on function parameter" \
"constexpr.c:7:1: error: illegal storage class on function" \
"constexpr.c:13:28: warning: implicit conversion from 'int' to '_BitInt' changes non-zero value from 4 to 0 [-Wconstant-conversion]" \

4 changes: 2 additions & 2 deletions test/cases/enum overflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ _Static_assert(H != C, "enumerator value was not truncated");
#define TESTS_SKIPPED 2
_Static_assert(A == 2147483647, "A");
// _Static_assert(B != 2147483648, "B");
_Static_assert(C == 4294967295UL, "C");
_Static_assert(C == -1, "C");
_Static_assert(D != 4294967296UL, "D");
_Static_assert(F != 9223372036854775807LL, "F");
// _Static_assert(G != 9223372036854775808ULL, "G");
_Static_assert(H == 18446744073709551615ULL, "H");
_Static_assert(H == -1, "H");

_Static_assert(H == C, "enumerator value was truncated");
#define EXPECTED_ERRORS "enum overflow.c:3:5: warning: overflow in enumeration value" \
Expand Down
1 change: 1 addition & 0 deletions test/cases/wide character constants.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ _Static_assert(sizeof(__CHAR16_TYPE__) == sizeof(u'A'), "");
_Static_assert(sizeof(__CHAR32_TYPE__) == sizeof(U'A'), "");

#define EXPECTED_ERRORS "wide character constants.c:9:27: error: character too large for enclosing character literal type" \
"wide character constants.c:9:20: warning: implicit conversion from 'int' to 'unsigned short' changes non-zero value from 131072 to 0 [-Wconstant-conversion]" \
"wide character constants.c:10:16: error: wide character literals may not contain multiple characters" \
"wide character constants.c:11:16: error: Unicode character literals may not contain multiple characters" \
"wide character constants.c:14:16: warning: multi-character character constant [-Wfour-char-constants]" \
Expand Down

0 comments on commit b61abd3

Please sign in to comment.