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

[CoreFoundation] Fetch a few static values lazily. #4924

Merged
merged 8 commits into from
Oct 9, 2018

Conversation

rolfbjarne
Copy link
Member

This avoids using static constructors, and also avoids fetching the values
unless they're needed.

This avoids using static constructors, and also avoids fetching the values
unless they're needed.
get {
if (kCFTypeArrayCallbacks_ptr_value == IntPtr.Zero)
kCFTypeArrayCallbacks_ptr_value = Dlfcn.GetIndirect (Libraries.CoreFoundation.Handle, "kCFTypeArrayCallBacks");
return kCFTypeArrayCallbacks_ptr_value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not move this into corefoundation.cs as [Field] ?
That will give us some free* introspection and xtro tests and less manual code to maintain.

Copy link
Member Author

Choose a reason for hiding this comment

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

That will call Dlfcn.GetIntPtr, which is incorrect for this (it needs to do Dlfcn.GetIndirect).

return;
try {
True = new CFBoolean (Dlfcn.GetIntPtr (handle, "kCFBooleanTrue"), false);
False = new CFBoolean (Dlfcn.GetIntPtr (handle, "kCFBooleanFalse"), false);
Copy link
Contributor

Choose a reason for hiding this comment

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

same, if we touch that code then I'd rather use the bindings file

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

get {
if (free_ptr == IntPtr.Zero)
free_ptr = Marshal.ReadIntPtr (Dlfcn.dlsym (Libraries.LibC.Handle, "_dispatch_data_destructor_free"));
return free_ptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

…tor.

Since the generator doesn't know about CFBoolean, bind as IntPtr instead, and
fix most callers to use the handle directly, instead of getting a CFBoolean
object and then immediately getting the handle.
@monojenkins
Copy link
Collaborator

Build failure
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

7 tests failed, 0 tests skipped, 73 tests passed.

Failed tests

  • introspection/Mac Unified: Failed (Test run failed.)
  • introspection/Mac Unified 32-bit: Failed (Test run failed.)
  • Xtro/Mac: Failed (Test run failed.)
  • introspection/iOS Unified 32-bits - simulator/Debug: Failed
  • introspection/iOS Unified 64-bits - simulator/Debug: Failed
  • introspection/tvOS - simulator/Debug: Failed
  • introspection/watchOS - simulator/Debug: Failed

@monojenkins
Copy link
Collaborator

Build failure
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

1 tests failed, 0 tests skipped, 80 tests passed.

Failed tests

  • Xtro/Mac: Failed (Test run failed.)

@monojenkins
Copy link
Collaborator

Build success
Build succeeded
API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)
Test run succeeded

@rolfbjarne rolfbjarne merged commit 65b21ee into xamarin:master Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants