Re: [PATCH 0/4] bitpos expansion summary reloaded

Jan Kratochvil
2012-10-23T19:11:00Z

Hi Siddhesh,

in the mail
http://sourceware.org/ml/gdb-patches/2012-09/msg00706.html

I have reported bitpos-patched GDB has the problem on line ada-valprint.c:700.

I do see this incomplete extension neither fixed nor listed in the
gcc-warnings.out.report file.

I see why, the line had the -Wconversion warning even before so it correctly
did not appear in the .report. But there is missing a bitpos extension now.

When I run -Wconversion by hand I do get:
ada-valprint.c: In function ‘ada_val_print_1’:
ada-valprint.c:700:68: error: conversion to ‘int’ from ‘long int’ may alter its value [-Werror=conversion]
while on the unpatched source tree one gets:
ada-valprint.c: In function ‘ada_val_print_1’:
ada-valprint.c:699:68: error: conversion to ‘int’ from ‘long int’ may alter its value [-Werror=conversion]

Unless one carefully checks all the uses of any extended variable - which
I was doing occasionally by hand but I understand it is very fragile to do in
all the cases by hand - this method does not work.

So there are these options:

(1) Check in the patchset as is while it is known not all type safety
regressions have been caught.

(2) Fix all -Wconversion warnings, either by cast or by type extension,
depending on the case. But this can be done anytime later.

The whole review effort improved the quality of (1) but it is still not a 100%
fix.

Fortunately (2) can be done in parts, (1) could not have been done in parts as
one cannot separate former -Wconversion vs. latter -Wconversion messages. But
it has been shown (1) cannot be done perfectly without (2).

Regards,
Jan