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

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Mon, 01 Oct 2012 07:20:48 +0200, Siddhesh Poyarekar wrote:
> However, how about committing these patches if
> they're correct and then doing a run with -Wconversion?  The patches
> are kinda unwieldy to maintain like this since it's quite painful
> to review the entire thing repeatedly and edit changelogs, etc. Besides,
> they should be safe for mainline since expansion of types should not
> pose a regression risk. I volunteer to do this of course.

I believe the only problem is with ChangeLogs.  When your patch is just
a proposal it is OK to review it without a ChangeLog, ChangeLog can be written
only after its approval when it is like in this case (a) a significant work to
write it and (b) it does not benefit the review process or self-review of the
patch which in this case it IMO does not.

The problem with checking it in partially is that this patch introduces new
conversion regressions which were not caught by splint such as:
  int len1 = TYPE_LENGTH (type);
  void *address1, *address2;
  int len2 = len1 + address1 - address2;
->
  LONGEST len1 = TYPE_LENGTH (type);
  void *address1, *address2;
  int len2 = len1 + address1 - address2;

After such patch gets checked in we will no longer find we need to expand also
LEN2.  Sure we cannot check in the type->LENGTH expansion itself due to the
same reason.

I believe (I may be wrong) there won't be too many new expansions from the
-Wconversion re-verification so you could just incrementally update the
existing patch incl. its ChangeLog.

I am thinking how to review it and I will have to create the patch again
anyway otherwise I do not know how to find out the annotations for it.


Thanks,
Jan