Test ext/intl/tests/calendar_clear_variation1.phpt fails with ICU 73.1

[andypost]

Description

The following code:

latest commit 976d7ed

Resulted in this output:

========DIFF========
     bool(true)
     bool(true)
     bool(false)
- float(1327813567000)
+ float(1330491967000)
     float(1327813567000)
========DONE========

But I expected this test to pass which means that month field is not cleared

php-src/ext/intl/tests/calendar_clear_variation1.phpt

Line 16 in 976d7ed

var_dump($intlcal->clear(IntlCalendar::FIELD_MONTH));

PHP Version

master

Operating System

Alpinelinux

[andypost]

Ref https://gitlab.alpinelinux.org/andypost/aports/-/merge_requests/3#note_303705

The test failed for all versions (8.0-8.2)

[andypost]

Implementation of the method got no changes and no changes at https://unicode-org.github.io/icu-docs/apidoc/dev/icu4c/deprecated.html

php-src/ext/intl/calendar/calendar_methods.cpp

Lines 440 to 462 in 976d7ed

	U_CFUNC PHP_FUNCTION(intlcal_clear)
	{
	zend_long field;
	bool field_is_null = 1;
	CALENDAR_METHOD_INIT_VARS;
	if (zend_parse_method_parameters(ZEND_NUM_ARGS(),
	getThis(), "O|l!", &object, Calendar_ce_ptr, &field, &field_is_null) == FAILURE) {
	RETURN_THROWS();
	}
	CALENDAR_METHOD_FETCH_OBJECT;
	if (field_is_null) {
	co->ucal->clear();
	} else {
	ZEND_VALUE_ERROR_INVALID_FIELD(field, 2);
	co->ucal->clear((UCalendarDateFields)field);
	}
	RETURN_TRUE;
	}

[andypost]

Used to build example app and found that clear() cpp method does not work in 72.1 but C-API stop working in 73.1

cal.cpp

#include "unicode/calendar.h"
#include "unicode/gregocal.h"
#include <stdio.h>

using namespace icu;

extern "C" void c_main();

void cpp_main()
{
  //cout << "CPP API " << endl;
  puts("CPP API");
  UErrorCode status = U_ZERO_ERROR;
  GregorianCalendar* gc = new GregorianCalendar(status);
  gc->set(2012, UCAL_FEBRUARY, 29);
  printf("before clear - year: %d, month: %d (%d in the implementation), day: %d\n",
           gc->get(UCAL_YEAR, status),
           gc->get(UCAL_MONTH, status) + 1,
           gc->get(UCAL_MONTH, status),
           gc->get(UCAL_DATE, status));
  gc->clear(UCAL_MONTH);
  gc->clear(UCAL_DATE);
  printf("after clear  - year: %d, month: %d (%d in the implementation), day: %d\n",
           gc->get(UCAL_YEAR, status),
           gc->get(UCAL_MONTH, status) + 1,
           gc->get(UCAL_MONTH, status),
           gc->get(UCAL_DATE, status));
  delete gc;
}

int main()
{
  cpp_main();

  c_main();

  return 0;
}

ccal.c

#include "unicode/ucal.h"
#include <stdio.h>

void c_main()
{
  puts("C API");
  UErrorCode status = U_ZERO_ERROR;
  int32_t i;
  UCalendar *cal = ucal_open(NULL, -1, NULL, UCAL_GREGORIAN, &status);
  ucal_set(cal, UCAL_YEAR, 2012);
  ucal_set(cal, UCAL_MONTH, UCAL_FEBRUARY);
  ucal_set(cal, UCAL_DATE, 29);
  printf("before clear - year: %d, month: %d (%d in the implementation), day: %d\n",
    ucal_get(cal, UCAL_YEAR, &status),
    ucal_get(cal, UCAL_MONTH, &status) + 1,
    ucal_get(cal, UCAL_MONTH, &status),
    ucal_get(cal, UCAL_DATE, &status));
  ucal_clearField(cal, UCAL_MONTH);
  printf("after clear  - year: %d, month: %d (%d in the implementation), day: %d\n",
    ucal_get(cal, UCAL_YEAR, &status),
    ucal_get(cal, UCAL_MONTH, &status) + 1,
    ucal_get(cal, UCAL_MONTH, &status),
    ucal_get(cal, UCAL_DATE, &status));
  ucal_close(cal);
}

[andypost]

The output for

test$ icu-config --version
73.1
test$ ./datecal
CPP API
before clear - year: 2012, month: 2 (1 in the implementation), day: 29
after clear  - year: 2012, month: 2 (1 in the implementation), day: 29
C API
before clear - year: 2012, month: 2 (1 in the implementation), day: 29
after clear  - year: 2012, month: 2 (1 in the implementation), day: 29

and previous (alpine:3.17)

test # icu-config --version
72.1
test # ./datecal 
CPP API
before clear - year: 2012, month: 2 (1 in the implementation), day: 29
after clear  - year: 2012, month: 2 (1 in the implementation), day: 29
C API
before clear - year: 2012, month: 2 (1 in the implementation), day: 29
after clear  - year: 2012, month: 1 (0 in the implementation), day: 29

[andypost]

Looks the only way to fix it is to use c++ API

[iluuu1994]

This fails on macOS on GitHub actions now too, probably due to an update of ICU. @andypost was this reported upstream?

[andypost]

Not yet as I failed to find docs for it(

[iluuu1994]

@andypost I haven't looked at this in detail yet, but I was confused that we're using the C++ API that is broken with 72.1 in your example.

php-src/ext/intl/calendar/calendar_methods.cpp

Lines 454 to 458 in 724af01

	co->ucal->clear();
	} else {
	ZEND_VALUE_ERROR_INVALID_FIELD(field, 2);
	co->ucal->clear((UCalendarDateFields)field);

But the test works fine for me with ICU 72.1. I'm not sure if some specific calendar condition makes it work/fail.

[andypost]

yes, the issue started to happen with 73.1 - Alpinelinux used to upgrade earlier so I hit it

[iluuu1994]

I think this is just a problem with the test. FIELD_ORDINAL_MONTH was introduced in ICU 73. If we're not clearing this field, month will be repopulated with the value from this field. I'll create a PR shortly.

[andypost]

I can't find any FIELD_ORDINAL_MONTH mentions, where it from?

[iluuu1994]

@andypost https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/ucal_8h.html#a02fe23bf33319052733c00c7a09ea912a1a565b5618fbb006c9b48b48f06a0167

It is also flagged as "draft API". It seems weird to me that introducing new fields can break clear, and even weirder that experimental fields can do so. I'm not sure what the right approach is here.

[iluuu1994]

I created https://unicode-org.atlassian.net/browse/ICU-22424 in the meantime.