[Bug bootstrap/14462] [3.5 Regression] a-calend.adb:396:33: warning: value not in range of type "Ada.Calendar.Day_Duration"

[dave at hiauly1 dot hia dot nrc dot ca]

------- Additional Comments From dave at hiauly1 dot hia dot nrc dot ca  
2004-03-11 23:12 -------
Subject: Re:  [3.5 Regression] a-calend.adb:396:33: warni

> This time, it is completely unimportant and even misleading: Ada bootstrap
> had
> been generally broken for a few days by another change, and fixed by
> the change in trans.c, as I explained in my message to gcc-patches,
> and as explained in the ChangeLog very clearly.
> 
> During the period Ada bootstrap was broken, you committed your change.
> 
> So knowing that trans.c is showing the new problem isn't going to help you
> at all in this case, since in fact the problem is not 'new' to trans.c, and
> would have appeared immediately, had Ada bootstrap not been broken.
> 
> Of course, having Ada enabled would have detected both (the general
> bootstrap failure and your recent regression) failures early.

@address@hidden  Please don't criticize others.  It just makes people
upset and is not constructive.

1) The ChangeLog entry for trans.c is not written using GCC standards.
Here it is:

2004-03-05  Richard Kenner  <address@hidden>

        * trans.c: Reflect GCC changes to fix bootstrap problem.
        Add warning for suspicious aliasing unchecked conversion.

ChangeLog entries are supposed to indicate the functions changed
and what was changed.  They are *not* supposed to contain a justification
for the change.  What the heck does "Reflect GCC changes" mean.  You
would never guess from this log entry that the change contains two
separate and independent changes.

2) One of the changes to trans.c was in fact a fix for the segmentation
fault in gnat1 that I reported in this PR:

@@ -5396,7 +5422,7 @@
 
    /* See if any non-NOTE insns were generated.  */
    for (insn = NEXT_INSN (insn); insn; insn = NEXT_INSN (insn))
-    if (GET_RTX_CLASS (GET_CODE (insn)) == 'i')
+    if (GET_RTX_CLASS (GET_CODE (insn)) == RTX_INSN)
       {
        result = 0;
        break;

3) If this patch had been reviewed, it wouldn't have been accepted.
The above code should be using the INSN_P macro so that that it isn't
dependent on the low level implementation of RTX classes.  The code
wouldn't have broken if it had used INSN_P.  Additionally, the trans.c
change uses whitespace in an inconsistent manner.

3) The change to trans.c did not in fact fix the bootstrap problem.
It had been broken further by this change committed after Paulo's change:

2004-03-04  Jan Hubicka  <address@hidden>

        * cfgcleanup.c (thread_jump): Update call of cselib_init.
        * cselib.c (cselib_record_memory): New static variable.
        (cselib_lookup_mem, cselib_record_set, cselib_record_sets):
        Give up on memories when asked for.
        (cselib_init): Accept new argument.
        * cselib.h (cselib_init): Update prototype.
        * gcse.c (local_cprop_pass): Update call of cselib_init.
        * loop.c (load_mems): Update call of cselib_init.
        * postreload.c (reload_cse_regs_1): Update call of cselib_init.
        * sched-deps.c (sched_analyze): Update call of cselib_init.

Dave


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14462