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

[ObjC] Name lookup in methods shouldn't allow shadowing types #9632

Open
wants to merge 1 commit into
base: stable/20240723
Choose a base branch
from

Conversation

apple-fcloutier
Copy link

Arguably as a bug, Clang has previously not mixed up Objective-C parameter names with types. This allows developers to write parameter names that should shadow type names, but don't. For instance:

@interface Foo
-(void)foo:(int)id bar:(id)name; // OK
@end

Commit 9778808 changed the way that parameters are parsed to bring it more in line with how C parameters are parsed, but it breaks the example above. Given an expectation that the change wouldn't introduce source breaks, this is not something we can go forward with.

977880... did this so that late-parsed attributes could reference Objective-C parameters. This change buffers Objective-C parameter info until after all parameters are parsed and turns them into parameter declarations before realizing late-parsed attributes instead.

Radar-ID: 139996306

Arguably as a bug, Clang has previously not mixed up Objective-C
parameter names with types. This allows developers to write
parameter names that _should_ shadow type names, but don't. For
instance:

    @interface Foo
    -(void)foo:(int)id bar:(id)name; // OK
    @EnD

Commit 9778808 changed the way
that parameters are parsed to bring it more in line with how C
parameters are parsed, but it breaks the example above. Given an
expectation that the change wouldn't introduce source breaks, this
is not something we can go forward with.

977880... did this so that late-parsed attributes could reference
Objective-C parameters. This change buffers Objective-C parameter
info until after all parameters are parsed and turns them into
parameter declarations before realizing late-parsed attributes
instead.

Radar-ID: 139996306
@apple-fcloutier apple-fcloutier self-assigned this Nov 19, 2024
@apple-fcloutier
Copy link
Author

@swift-ci please test llvm

@apple-fcloutier
Copy link
Author

@swift-ci test

@AreaZR
Copy link

AreaZR commented Nov 19, 2024

I will do one better, merge this into 19.x

@apple-fcloutier
Copy link
Author

I can merge to any other branch that llvm#113745 made it to, but as a guest compiler contributor from SEAR, I don't know what branch any given version number matches (or their position in the forwarding graph, for that matter). Please let me know what that would be.

@AreaZR
Copy link

AreaZR commented Nov 20, 2024

Actually, doing it here is fine since it's not passing right now on 19.x

@apple-fcloutier
Copy link
Author

/Users/ec2-user/jenkins/workspace/apple-llvm-project-pr-macos/branch-stable/20240723/sourcekit-lsp/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift:318:107: error: cannot convert value of type 'URL' to expected argument type 'TSCAbsolutePath' (aka 'AbsolutePath')
316 |       )
317 |     } else if let scratchDirectory = options.swiftPMOrDefault.scratchPath,
318 |       let scratchDirectoryPath = try? AbsolutePath(validating: scratchDirectory, relativeTo: AbsolutePath(projectRoot))
    |                                                                                                           `- error: cannot convert value of type 'URL' to expected argument type 'TSCAbsolutePath' (aka 'AbsolutePath')
319 |     {
320 |       location.scratchDirectory = scratchDirectoryPath

Pretty sure this isn't my fault; do you know who to pull in?

@AreaZR
Copy link

AreaZR commented Nov 20, 2024

Sadly no.

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