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

Move to objc2 #1461

Merged
merged 2 commits into from
Feb 6, 2023
Merged

Move to objc2 #1461

merged 2 commits into from
Feb 6, 2023

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Sep 9, 2022

Helps with following Cocoa's memory management rules (for example, #1453 wouldn't have happened with this) - and while I was at it, I fixed a few C enum definitions (objc2's verify_message feature is a great help with that).

See commit messages for details.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality

@kchibisov
Copy link
Member

Ah, you'd need a CI update for that? Could you also rebase?

@madsmtm madsmtm mentioned this pull request Sep 9, 2022
1 task
@madsmtm
Copy link
Member Author

madsmtm commented Sep 9, 2022

I moved the required MSRV bump to #1464

@madsmtm
Copy link
Member Author

madsmtm commented Sep 11, 2022

Will rebase after #1465 probably

@madsmtm madsmtm mentioned this pull request Sep 21, 2022
4 tasks
@kchibisov
Copy link
Member

I'm not sure how I should go forward with it, since the change is more or less stylistic right now. I'd wait for objc2 to release first. Don't want to release anything glutin with unstable crates.

@madsmtm
Copy link
Member Author

madsmtm commented Oct 4, 2022

Fair - but then you should probably merge #1474 (otherwise glutin doesn't compile on macOS aarch64)

@kchibisov
Copy link
Member

kchibisov commented Oct 4, 2022

I think it's aliased to 0 anyway?

Ah, I see, can't reopen though, should resend PR.

@kchibisov
Copy link
Member

@madsmtm would you mind updating the branch? I'm fine with having the same objc2 as in winit.

@madsmtm
Copy link
Member Author

madsmtm commented Feb 3, 2023

Cool, I'll probably get to it at some point in the near future then

This is a breaking change
In particular, this fixes:
- UB in -[NSOpenGLPixelFormat getValues:forAttribute:forVirtualScreen:],
cocoa mistakenly passed u64 where they should pass u32
- UB in -[NSOpenGLContext setValues:forParameter:], cocoa mistakenly
passed u64 where they should pass usize
- UB if -[NSOpenGLContext initWithFormat:shareContext:] returned NULL
@kchibisov
Copy link
Member

Seems like nothing really changed, so rebase went without issues.

@kchibisov kchibisov merged commit 0f06405 into master Feb 6, 2023
@kchibisov kchibisov deleted the objc2 branch February 6, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants