Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose zendi_try_get_long() function via a public API #10175

Merged
merged 1 commit into from Jun 19, 2023

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Dec 28, 2022

We probably should be exposing zendi_try_convert_scalar_to_number() as well as this would make the implementation of #10161 slightly easier because I find it kinda insane that the APIs we provide to get certain types just blindly accept arrays and non-representable objects by making them return somewhat arbitrary values even though for int/float we consider this UB.

@Girgias
Copy link
Member Author

Girgias commented Dec 28, 2022

Erf seem there are still some issues with clean sections on IO tests after I added some:

BORK file() on a path containing .. and .
array(3) {
  [0]=>
  string(7) "Line 1
"
  [1]=>
  string(7) "Line 2
"
  [2]=>
  string(6) "Line 3"
}
file() on a path containing .. with invalid directories
array(3) {
  [0]=>
  string(7) "Line 1
"
  [1]=>
  string(7) "Line 2
"
  [2]=>
  string(6) "Line 3"
}
file() on a relative path from a different working directory
array(3) {
  [0]=>
  string(7) "Line 1
"
  [1]=>
  string(7) "Line 2
"
  [2]=>
  string(6) "Line 3"
} [C:\projects\php-src\ext\standard\tests\file\file_variation5-win32.phpt] reason: invalid output from CLEAN
=====================================================================
TIME END 2022-12-28 17:05:44

cmb69 added a commit to cmb69/php-src that referenced this pull request Dec 30, 2022
Each test should use its own temporary filenames to avoid issues when
the tests are executed in parallel[1].  We also silence the `unlink()`
calls in the CLEAN section just in case.

And while we're at it, we also remove the erroneous comment; there is
no symlinking involved for the Windows test variants.

[1] <php#10175 (comment)>
cmb69 added a commit that referenced this pull request Dec 30, 2022
Each test should use its own temporary filenames to avoid issues when
the tests are executed in parallel[1].  We also silence the `unlink()`
calls in the CLEAN section just in case.

And while we're at it, we also remove the erroneous comment; there is
no symlinking involved for the Windows test variants.

[1] <#10175 (comment)>

Closes GH-10189.
Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Girgias Girgias merged commit ea8f934 into php:master Jun 19, 2023
12 of 13 checks passed
@Girgias Girgias deleted the expose-zendi-try-get-long branch June 19, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants