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

[logos] Support multi-line block arguments in %orig calls #114

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

EthanArbuckle
Copy link

What does this implement/fix? Explain your changes.

This PR adds the ability to specify inline blocks spanning multiple lines as arguments when calling %orig(...), which was previously not supported.

Example %orig(...) usages that are now supported:

%orig(^{
    NSLog(@"foo");
});
%orig(arg1, arg2, ^(NSString *blockArg1) {
    NSLog(@"block arg: %@", blockArg1);
});
- (void)methodWithComplexBlock:(void (^)(NSString *, NSError *))completion {
    %orig(^(NSString *result, NSError *error) {
        if (error) {
            NSLog(@"Error: %@", error);
        }
        completion(result, error);
    });
}

Does this close any currently open issues?

This PR closes an issue that's been open for 10y: [#6] "Logos errors out on calling %orig with an inline block with multiple lines."

Any relevant logs, error output, etc?

N/A

Any other comments?

  • This maintains backwards compatibility with existing Logos syntax. Developers do not need to modify their projects/code to accommodate this change.
  • Using logos directives (%orig, %log, etc.) inside of a block passed to %orig(...) is currently not supported.
    • Attempting to do so will result in the following error: Tweak.x:4: error: Logos cannot be used within arguments to %orig.

Example of unsupported usage:

%orig(arg1, arg2, ^(NSString *blockArg1) {
    %orig(arg1, arg2, nil);
});

Tweak.x:4: error: Logos cannot be used within arguments to %orig

Where has this been tested?

Operating System: macOS Sonoma, iOS 14, iOS 18, Ubuntu 22.0.4

Platform: arm64, x86_64

Target Platform: iOS, iOS Simulator

Toolchain Version: Xcode 15.4

SDK Version: iOS 14, iOS 15, iOS 16.5

Unit Testing

AFAIK, Logos (and Theos) lack official unit testing. This PR changes core behavior of the parser and has the potential to introduce significant breakage; this requires thorough testing to ensure reliability and backwards compatibility. To address this concern, I created a separate branch containing unit tests for my changes as well as some unrelated core behavior, as the project lacks these otherwise.

Coverage of these tests include, but is not limited to:

  1. Basic %orig functionality to ensure backwards compatibility.
  2. New multi-line block parsing capabilities.
  3. Error handling for invalid syntax.
  4. Edge cases such as complex argument structures nested %orig usage.

These tests validate the correctness of the current implementation and provide a safety net for future changes (helping prevent regressions). All of the tests I created can be found here.

Direct link to the tests for %orig behavior.

I have Github Actions setup to run these tests (in my fork) -- they're currently passing: Latest Github Action run.

Copy link
Member

@uroboro uroboro left a comment

Choose a reason for hiding this comment

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

I finally had time to go over this and understand it.

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.

2 participants