This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Patch: java.util.Calendar


Jeff> Which test?  I'm seeing:

I see this:

    FAIL: gnu.testlet.java.util.Calendar.simple (number 3)
    got 04/30/2001 but expected 05/01/2001
    FAIL: gnu.testlet.java.util.Calendar.simple (number 4)
    got weekday = 2 but expected weekday = 3
    FAIL: gnu.testlet.java.util.Calendar.simple (number 5)
    got 05/30/2001 but expected 06/01/2001
    FAIL: gnu.testlet.java.util.Calendar.simple (number 6)
    got weekday = 4 but expected weekday = 6

The failures you cite are fixed by other parts of my patch.  I also
just fixed the DateFormatSymbols problem.  It was due to old
pre-Locale code still lying around.  (This fix isn't in the appended
patch.)

The new failures I'm seeing are probably the result of my tinkering
with GregorianCalendar.computeFields().  I'll probably look at them
tomorrow morning.

Until I rewrote this method, all the Mauve Calendar tests were
failing.

I think the add() method is still wrong, since it doesn't take into
account actual maximums.  For instance if you watch the first failure
above in the debugger you can see that the DATE field of the Calendar
object is set to 31.  You might still get the right result from
getTime(), but if you use get(DATE) you should see the wrong answer
(it should be 1, with the month incremented).

This problem I won't fix.  Maybe merging with Classpath is the only
sane thing at this point?

>> I tracked down my particular failure to the use of localtime in
>> GregorianCalendar.computeFields.  What timezone are you in?  The
>> failure is timezone dependent.

Jeff> I'm in PDT.

Bummer.  If you were further east then I would understand why it
worked for you and not me.  I guess some other part of my patch is
affecting your patch somehow.

I've appended my current patch (which includes yours) in case you want
to play with it.

Tom

Index: java/text/SimpleDateFormat.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/text/SimpleDateFormat.java,v
retrieving revision 1.8.4.1
diff -u -r1.8.4.1 SimpleDateFormat.java
--- SimpleDateFormat.java	2001/04/20 09:41:11	1.8.4.1
+++ SimpleDateFormat.java	2001/04/27 03:57:54
@@ -62,8 +62,7 @@
 
   private transient Vector tokens;
   private DateFormatSymbols formatData;  // formatData
-  private Date defaultCenturyStart = 
-    new Date(System.currentTimeMillis() - (80*365*24*60*60*1000));
+  private Date defaultCenturyStart = computeCenturyStart ();
   private String pattern;
   private int serialVersionOnStream = 1; // 0 indicates JDK1.1.3 or earlier
   private static final long serialVersionUID = 4774881970558875024L;
@@ -79,8 +78,7 @@
     stream.defaultReadObject();
     if (serialVersionOnStream < 1)
       {
-        defaultCenturyStart =
-	  new Date(System.currentTimeMillis() - (80*365*24*60*60*1000));
+        defaultCenturyStart = computeCenturyStart ();
 	serialVersionOnStream = 1;
       }
 
@@ -161,11 +159,14 @@
     super();
     Locale locale = Locale.getDefault();
     calendar = new GregorianCalendar(locale);
+    calendar.clear ();
     tokens = new Vector();
     formatData = new DateFormatSymbols(locale);
-    pattern = formatData.dateFormats[DEFAULT]+' '+formatData.timeFormats[DEFAULT];
+    pattern = (formatData.dateFormats[DEFAULT] + ' '
+	       + formatData.timeFormats[DEFAULT]);
     compileFormat(pattern);
     numberFormat = NumberFormat.getInstance(locale);
+    numberFormat.setGroupingUsed (false);
   }
   
   /**
@@ -185,20 +186,24 @@
   {
     super();
     calendar = new GregorianCalendar(locale);
+    calendar.clear ();
     tokens = new Vector();
     formatData = new DateFormatSymbols(locale);
     compileFormat(pattern);
     this.pattern = pattern;
     numberFormat = NumberFormat.getInstance(locale);
+    numberFormat.setGroupingUsed (false);
   }
 
   /**
    * Creates a date formatter using the specified pattern. The
    * specified DateFormatSymbols will be used when formatting.
    */
-  public SimpleDateFormat(String pattern, DateFormatSymbols formatData) {
+  public SimpleDateFormat(String pattern, DateFormatSymbols formatData)
+  {
     super();
     calendar = new GregorianCalendar();
+    calendar.clear ();
     // FIXME: XXX: Is it really necessary to set the timezone?
     // The Calendar constructor is supposed to take care of this.
     calendar.setTimeZone(TimeZone.getDefault());
@@ -207,6 +212,7 @@
     compileFormat(pattern);
     this.pattern = pattern;
     numberFormat = NumberFormat.getInstance();
+    numberFormat.setGroupingUsed (false);
   }
 
   // What is the difference between localized and unlocalized?  The
@@ -377,7 +383,8 @@
    * appending to the specified StringBuffer.  The input StringBuffer
    * is returned as output for convenience.
    */
-  public StringBuffer format(Date date, StringBuffer buffer, FieldPosition pos) {
+  public StringBuffer format(Date date, StringBuffer buffer, FieldPosition pos)
+  {
     String temp;
     Calendar theCalendar = (Calendar) calendar.clone();
     theCalendar.setTime(date);
@@ -507,7 +514,10 @@
     int fmt_index = 0;
     int fmt_max = pattern.length();
 
-    calendar.clear();
+    // We copy the Calendar because if we don't we will modify it and
+    // then this.equals() will no longer have the desired result.
+    Calendar theCalendar = (Calendar) calendar.clone ();
+    theCalendar.clear();
     int quote_start = -1;
     for (; fmt_index < fmt_max; ++fmt_index)
       {
@@ -642,11 +652,13 @@
 		  }
 		if (k != strings.length)
 		  {
+		    found_zone = true;
 		    if (k > 2)
 		      ;		// FIXME: dst.
 		    zone_number = 0; // FIXME: dst.
 		    // FIXME: raw offset to SimpleTimeZone const.
-		    calendar.setTimeZone(new SimpleTimeZone (1, strings[0]));
+		    theCalendar.setTimeZone (new SimpleTimeZone (1,
+								 strings[0]));
 		    pos.setIndex(index + strings[k].length());
 		    break;
 		  }
@@ -692,18 +704,57 @@
 	else
 	  value = zone_number;
 
+	if (calendar_field == Calendar.HOUR)
+	  value %= 12;
+
 	// Assign the value and move on.
-	calendar.set(calendar_field, value);
+	theCalendar.set(calendar_field, value);
+
+	// If we're setting HOUR then take AM_PM into account, and
+	// vice versa.  Note that if we're setting HOUR_OF_DAY then we
+	// don't do any special processing.  FIXME: we only do this at
+	// all because of deficiencies in our Calendar implementation.
+	if ((calendar_field == Calendar.HOUR
+	     || calendar_field == Calendar.AM_PM)
+	    && theCalendar.isSet (Calendar.HOUR))
+	  {
+	    int my_hour = theCalendar.get (Calendar.HOUR);
+	    if (theCalendar.isSet (Calendar.AM_PM))
+	      {
+		if (theCalendar.get (Calendar.AM_PM) == Calendar.PM)
+		  my_hour += 12;
+		else
+		  my_hour %= 12;
+	      }
+	    theCalendar.set (Calendar.HOUR_OF_DAY, my_hour);
+	  }
       }
 
     try
       {
-        return calendar.getTime();
+        return theCalendar.getTime();
       }
     catch (IllegalArgumentException x)
       {
         pos.setErrorIndex(pos.getIndex());
 	return null;
       }
+  }
+
+  // Compute the start of the current century as defined by
+  // get2DigitYearStart.
+  private Date computeCenturyStart ()
+  {
+    // Compute the current year.  We assume a year has 365 days.  Then
+    // compute 80 years ago, and finally reconstruct the number of
+    // milliseconds.  We do this computation in this strange way
+    // because it lets us easily truncate the milliseconds, seconds,
+    // etc, which don't matter and which confuse
+    // SimpleDateFormat.equals().
+    long now = System.currentTimeMillis ();
+    now /= 365L * 24L * 60L * 60L * 1000L;
+    now -= 80;
+    now *= 365L * 24L * 60L * 60L * 1000L;
+    return new Date (now);
   }
 }
Index: java/util/Calendar.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/Calendar.java,v
retrieving revision 1.8
diff -u -r1.8 Calendar.java
--- Calendar.java	2000/12/28 05:55:56	1.8
+++ Calendar.java	2001/04/27 03:57:54
@@ -1,5 +1,5 @@
 /* java.util.Calendar
-   Copyright (C) 1998, 1999, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1998, 1999, 2000, 2001 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -54,7 +54,7 @@
  *
  * When computing the date from time fields, it may happen, that there
  * are either two few fields set, or some fields are inconsistent.  This
- * cases will handled in a calender specific way.  Missing fields are
+ * cases will handled in a calendar specific way.  Missing fields are
  * replaced by the fields of the epoch: 1970 January 1 00:00. <br>
  *
  * To understand, how the day of year is computed out of the fields
@@ -356,7 +356,7 @@
   private static final String bundleName = "gnu.java.locale.Calendar";
 
   /**
-   * Constructs a new Calender with the default time zone and the default
+   * Constructs a new Calendar with the default time zone and the default
    * locale.
    */
   protected Calendar()
@@ -365,7 +365,7 @@
   }
 
   /**
-   * Constructs a new Calender with the given time zone and the given
+   * Constructs a new Calendar with the given time zone and the given
    * locale.
    * @param zone a time zone.
    * @param locale a locale.
@@ -483,7 +483,7 @@
   }
 
   /**
-   * Sets this Calender's time to the given Date.  All time fields
+   * Sets this Calendar's time to the given Date.  All time fields
    * are invalidated by this method.
    */
   public final void setTime(Date date)
@@ -503,7 +503,7 @@
   }
 
   /**
-   * Sets this Calender's time to the given Time.  All time fields
+   * Sets this Calendar's time to the given Time.  All time fields
    * are invalidated by this method.
    * @param time the time in milliseconds since the epoch
    */
@@ -522,6 +522,9 @@
    */
   public final int get(int field)
   {
+    // If the requested field is invalid, force all fields to be recomputed.
+    if (!isSet[field])
+      areFieldsSet = false;
     complete();
     return fields[field];
   }
@@ -551,6 +554,29 @@
     isTimeSet = false;
     fields[field] = value;
     isSet[field] = true;
+    switch (field)
+      {
+      case YEAR:
+      case MONTH:
+      case DATE:
+	isSet[WEEK_OF_YEAR] = false;
+	isSet[DAY_OF_YEAR] = false;
+	isSet[WEEK_OF_MONTH] = false;
+	isSet[DAY_OF_WEEK] = false;
+	isSet[DAY_OF_WEEK_IN_MONTH] = false;
+	break;
+      case AM_PM:
+	isSet[HOUR_OF_DAY] = false;
+	break;
+      case HOUR_OF_DAY:
+	isSet[AM_PM] = false;
+	isSet[HOUR] = false;
+	break;
+      case HOUR:
+	isSet[AM_PM] = false;
+	isSet[HOUR_OF_DAY] = false;
+	break;
+      }
   }
 
   /**
@@ -568,6 +594,11 @@
     fields[MONTH] = month;
     fields[DATE] = date;
     isSet[YEAR] = isSet[MONTH] = isSet[DATE] = true;
+    isSet[WEEK_OF_YEAR] = false;
+    isSet[DAY_OF_YEAR] = false;
+    isSet[WEEK_OF_MONTH] = false;
+    isSet[DAY_OF_WEEK] = false;
+    isSet[DAY_OF_WEEK_IN_MONTH] = false;
   }
 
   /**
@@ -584,6 +615,8 @@
     fields[HOUR_OF_DAY] = hour;
     fields[MINUTE] = minute;
     isSet[HOUR_OF_DAY] = isSet[MINUTE] = true;
+    isSet[AM_PM] = false;
+    isSet[HOUR] = false;
   }
 
   /**
@@ -611,7 +644,10 @@
     isTimeSet = false;
     areFieldsSet = false;
     for (int i = 0; i < FIELD_COUNT; i++)
-      isSet[i] = false;
+      {
+	isSet[i] = false;
+	fields[i] = 0;
+      }
   }
 
   /**
@@ -623,6 +659,7 @@
     isTimeSet = false;
     areFieldsSet = false;
     isSet[field] = false;
+    fields[field] = 0;
   }
 
   /**
@@ -647,7 +684,7 @@
   }
 
   /**
-   * Compares the given calender with this.  
+   * Compares the given calendar with this.  
    * @param o the object to that we should compare.
    * @return true, if the given object is a calendar, that represents
    * the same time (but doesn't neccessary have the same fields).
@@ -670,10 +707,10 @@
   }
 
   /**
-   * Compares the given calender with this.  
+   * Compares the given calendar with this.  
    * @param o the object to that we should compare.
    * @return true, if the given object is a calendar, and this calendar
-   * represents a smaller time than the calender o.
+   * represents a smaller time than the calendar o.
    * @exception ClassCastException if o is not an calendar.
    * @since JDK1.2 you don't need to override this method
    */
@@ -683,10 +720,10 @@
   }
 
   /**
-   * Compares the given calender with this.  
+   * Compares the given calendar with this.  
    * @param o the object to that we should compare.
    * @return true, if the given object is a calendar, and this calendar
-   * represents a bigger time than the calender o.
+   * represents a bigger time than the calendar o.
    * @exception ClassCastException if o is not an calendar.
    * @since JDK1.2 you don't need to override this method
    */
Index: java/util/GregorianCalendar.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/GregorianCalendar.java,v
retrieving revision 1.8
diff -u -r1.8 GregorianCalendar.java
--- GregorianCalendar.java	2000/12/28 05:55:56	1.8
+++ GregorianCalendar.java	2001/04/27 03:57:54
@@ -1,4 +1,4 @@
-/* Copyright (C) 1998, 1999, 2000  Free Software Foundation
+/* Copyright (C) 1998, 1999, 2000, 2001  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -134,7 +134,14 @@
 
   private final void setDefaultTime ()
   {
-    setTimeInMillis (System.currentTimeMillis());
+    // The docs say that we only need to set the year, month, date,
+    // hour, minute, and second.  And indeed if we try to set the
+    // milliseconds here then confusion results when using Calendar --
+    // there isn't a particularly intuitive way to clear the
+    // milliseconds.
+    long now = System.currentTimeMillis ();
+    now /= 1000;
+    setTimeInMillis (now * 1000);
   }
 
   public int getMinimum(int calfield) { return mins[calfield]; }
@@ -148,15 +155,15 @@
 
   public void add (int fld, int amount)
   {
-    if (fld >= ZONE_OFFSET)
+    if (fld < 0 || fld >= ZONE_OFFSET)
       throw new IllegalArgumentException("bad field to add");
-    fields[fld] += amount;
-    adjust(fld);
+    set (fld, get (fld) + amount);
+    adjust (fld);
   }
 
   public void roll (int fld, boolean up)
   {
-    if (fld >= ZONE_OFFSET)
+    if (fld < 0 || fld >= ZONE_OFFSET)
       throw new IllegalArgumentException("bad field to roll");
 
     int old = fields[fld];
@@ -174,26 +181,32 @@
 
   private void adjust (int fld)
   {
-    int value = fields[fld];
+    int value = get (fld);
     int radix = maxs[fld] + 1;
     switch (fld)
       {
       case MONTH:
+      case DATE:
+      case HOUR_OF_DAY:
+      case MINUTE:
       case SECOND:
       case MILLISECOND:
 	if (value >= radix)
 	  {
 	    int next = value / radix;
-	    fields[fld] = value - radix * next;
-	    fields[fld - 1] += next;
-	    adjust(fld - 1);
+	    set (fld, value - radix * next);
+	    if (next > 0)
+	      {
+		set (fld - 1, get (fld - 1) + next);
+		adjust (fld - 1);
+	      }
 	  }
 	else if (value < 0) // min[fld]
 	  {
 	    int next = (value - radix - 1) / radix;
-	    fields[fld] = value - radix * next;
-	    fields[fld - 1] += next;
-            adjust(fld - 1);
+	    set (fld, value - radix * next);
+	    set (fld - 1, get (fld - 1) + next);
+            adjust (fld - 1);
 	  }
 	break;
       }
Index: java/util/SimpleTimeZone.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/SimpleTimeZone.java,v
retrieving revision 1.6
diff -u -r1.6 SimpleTimeZone.java
--- SimpleTimeZone.java	2000/11/22 11:59:59	1.6
+++ SimpleTimeZone.java	2001/04/27 03:57:56
@@ -1,5 +1,5 @@
 /* java.util.SimpleTimeZone
-   Copyright (C) 1998, 1999, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1998, 1999, 2000, 2001 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -518,7 +518,7 @@
    * @param month The month, zero based; use one of the Calendar constants.
    * @param year  The year.
    */
-  private int getDaysInMonth(int month, int year)
+  static int getDaysInMonth(int month, int year)
   {
     // Most of this is copied from GregorianCalendar.getActualMaximum()
     if (month == Calendar.FEBRUARY)
Index: java/util/natGregorianCalendar.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/util/natGregorianCalendar.cc,v
retrieving revision 1.8.4.1
diff -u -r1.8.4.1 natGregorianCalendar.cc
--- natGregorianCalendar.cc	2001/04/20 09:41:11	1.8.4.1
+++ natGregorianCalendar.cc	2001/04/27 03:57:56
@@ -16,6 +16,8 @@
 #include <java/util/TimeZone.h>
 #include <java/util/GregorianCalendar.h>
 #include <java/lang/IllegalArgumentException.h>
+#include <java/util/SimpleTimeZone.h>
+#include <java/lang/Class.h>
 #include <time.h>
 
 void
@@ -81,7 +83,7 @@
   // Adjust for local timezone (introduced by mktime) and our
   // timezone.
 #if defined (STRUCT_TM_HAS_GMTOFF)
-  t -= tim.tm_gmtoff;
+  t += tim.tm_gmtoff;
 #elif defined (HAVE_TIMEZONE)
   t += timezone;
 #endif
@@ -95,6 +97,24 @@
   isTimeSet = true;
 }
 
+static void
+my_gmtime (time_t t, struct tm *timp)
+{
+  // FIXME: None of the standard C library access to the ECOS calendar
+  // is yet available.
+#ifdef ECOS
+  memset (timp, 0, sizeof *timp); 
+#else
+#if defined(__JV_POSIX_THREADS__) && defined(HAVE_GMTIME_R)
+  gmtime_r (&t, timp);
+#else
+  // Get global lock (because gmtime uses a global buffer).  FIXME
+  *timp = *(struct tm *) gmtime (&t);
+  // Release global lock.  FIXME
+#endif
+#endif /* ECOS */
+}
+
 void
 java::util::GregorianCalendar::computeFields ()
 {
@@ -105,36 +125,30 @@
       t--;
       millis = t - 1000 * t;
     }
-  elements(fields)[MILLISECOND] = millis;
   struct tm tim;
-  java::util::TimeZone *zone = getTimeZone ();
+  my_gmtime (t, &tim);
 
-  // FIXME: None of the standard C library access to the ECOS calendar
-  // is yet available.
-#ifdef ECOS
-  memset (&tim, 0, sizeof tim); 
-#else
-  if (zone->getRawOffset() == 0 || ! zone->useDaylightTime())
-    {
-#if defined(__JV_POSIX_THREADS__) && defined(HAVE_GMTIME_R)
-      gmtime_r (&t, &tim);
-#else
-      // Get global lock (because gmtime uses a global buffer).  FIXME
-      tim = *(struct tm*) gmtime (&t);
-      // Release global lock.  FIXME
-#endif
-    }
-  else
+  // Account for time zone by adjusting fields.
+  // We do this in a funny way: we adjust the time by the timezone
+  // info, taking DST into account, and then we re-run gmtime.
+  // This gets us the "right" results which we then use to compute all
+  // the fields.  This is ugly, and perhaps slow (I don't know), but
+  // the logic is definitely simpler than the alternative.
+
+  java::util::TimeZone *zone = getTimeZone ();
+  int offset = zone->getOffset (AD, 1900 + tim.tm_year, tim.tm_mon,
+				tim.tm_mday, tim.tm_wday + 1, millis);
+  t = (time + offset) / 1000;
+  millis = (time + offset) % 1000;
+  if (t < 0 && millis != 0)
     {
-#if defined(__JV_POSIX_THREADS__) && defined(HAVE_LOCALTIME_R)
-      localtime_r (&t, &tim);
-#else
-      // Get global lock (because localtime uses a global buffer).  FIXME
-      tim = *(struct tm*) localtime (&t);
-      // Release global lock.  FIXME
-#endif
+      t--;
+      millis = t - 1000 * t;
     }
-#endif /* ECOS */
+  elements(fields)[MILLISECOND] = millis;
+
+  my_gmtime (t, &tim);
+
   elements(fields)[SECOND] = tim.tm_sec;
   elements(fields)[MINUTE] = tim.tm_min;
   elements(fields)[HOUR_OF_DAY] = tim.tm_hour;
@@ -151,8 +165,16 @@
   elements(fields)[WEEK_OF_YEAR]
     = (tim.tm_yday + 7 + (5 - tim.tm_wday + getFirstDayOfWeek()) % 7) / 7;
   elements(fields)[ERA] = AD;
-  elements(fields)[DST_OFFSET] = tim.tm_isdst <= 0 ? 0 : 60*60*1000;
-  elements(fields)[ZONE_OFFSET] = getTimeZone()->getRawOffset();
+
+  int dst = 0;
+  if (SimpleTimeZone:: class$.isInstance (zone))
+    {
+      SimpleTimeZone *stz = reinterpret_cast<SimpleTimeZone *> (zone);
+      dst = stz->getDSTSavings ();
+    }
+  elements(fields)[DST_OFFSET] = dst;
+
+  elements(fields)[ZONE_OFFSET] = zone->getRawOffset();
   areFieldsSet = true;
   for (int i = 0; i < FIELD_COUNT; i++)
     elements(isSet__)[i] = true;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]