[clang] release/19.x: [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) (PR #116861)
https://github.com/AreaZR reopened https://github.com/llvm/llvm-project/pull/116861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] release/19.x: [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) (PR #116861)
https://github.com/AreaZR edited https://github.com/llvm/llvm-project/pull/116861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] release/19.x: [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) (PR #116861)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Rose (AreaZR) Changes 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 97788089988a2ace63d717cadbcfe3443f380f9c 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. 97788089988a2ace63d717cadbcfe3443f380f9c 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 (cherry picked from commit 8bdf13b11638d2f3e6792a573fc2be830a03790a) --- Full diff: https://github.com/llvm/llvm-project/pull/116861.diff 2 Files Affected: - (modified) clang/lib/Parse/ParseObjc.cpp (+13-4) - (added) clang/test/SemaObjC/method-param-named-id.m (+7) ``diff diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index e69fa152481982..ddf59bd748f343 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -1454,7 +1454,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, SmallVector KeyIdents; SmallVector KeyLocs; - SmallVector ObjCParamInfo; + SmallVector ArgInfos; ParseScope PrototypeScope(this, Scope::FunctionPrototypeScope | Scope::FunctionDeclarationScope | Scope::DeclScope); @@ -1495,9 +1495,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, ArgInfo.NameLoc = Tok.getLocation(); ConsumeToken(); // Eat the identifier. -ParmVarDecl *Param = Actions.ObjC().ActOnMethodParmDeclaration( -getCurScope(), ArgInfo, ObjCParamInfo.size(), MethodDefinition); -ObjCParamInfo.push_back(Param); +ArgInfos.push_back(ArgInfo); KeyIdents.push_back(SelIdent); KeyLocs.push_back(selLoc); @@ -1557,6 +1555,17 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, nullptr)); } + // Turn ArgInfos into parameters. This must happen after parsing all + // parameters for bug compatibility with previous versions of Clang. (For + // instance, if a method declares a parameter called "id", that parameter must + // not shadow the "id" type.) + SmallVector ObjCParamInfo; + for (auto &ArgInfo : ArgInfos) { +ParmVarDecl *Param = Actions.ObjC().ActOnMethodParmDeclaration( +getCurScope(), ArgInfo, ObjCParamInfo.size(), MethodDefinition); +ObjCParamInfo.push_back(Param); + } + // FIXME: Add support for optional parameter list... // If attributes exist after the method, parse them. MaybeParseAttributes(PAKM_CXX11 | (getLangOpts().ObjC ? PAKM_GNU : 0), diff --git a/clang/test/SemaObjC/method-param-named-id.m b/clang/test/SemaObjC/method-param-named-id.m new file mode 100644 index 00..8269c31116c32d --- /dev/null +++ b/clang/test/SemaObjC/method-param-named-id.m @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s + + +@interface Foo +-(void)paramNamedID:(int)id usesIDType:(id)notShadowed; +-(void)paramNamedID:(int)id, id notShadowed; // expected-warning{{use of C-style parameters in Objective-C method declarations is deprecated}} +@end `` https://github.com/llvm/llvm-project/pull/116861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] release/19.x: [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) (PR #116861)
AreaZR wrote: Requesting it be merged this because it is being merged into the apple branch based off 19.x https://github.com/llvm/llvm-project/pull/116861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] release/19.x: [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) (PR #116861)
https://github.com/AreaZR closed https://github.com/llvm/llvm-project/pull/116861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] release/19.x: [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) (PR #116861)
https://github.com/AreaZR created https://github.com/llvm/llvm-project/pull/116861 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 97788089988a2ace63d717cadbcfe3443f380f9c 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. 97788089988a2ace63d717cadbcfe3443f380f9c 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 (cherry picked from commit 8bdf13b11638d2f3e6792a573fc2be830a03790a) >From 419bd5babd2e386306e8c6a38aba1ad2b1f55270 Mon Sep 17 00:00:00 2001 From: apple-fcloutier <75502309+apple-fclout...@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:33:35 -0800 Subject: [PATCH] [ObjC] Name lookup in methods shouldn't allow shadowing types (#116683) 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 97788089988a2ace63d717cadbcfe3443f380f9c 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. 97788089988a2ace63d717cadbcfe3443f380f9c 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 (cherry picked from commit 8bdf13b11638d2f3e6792a573fc2be830a03790a) --- clang/lib/Parse/ParseObjc.cpp | 17 + clang/test/SemaObjC/method-param-named-id.m | 7 +++ 2 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 clang/test/SemaObjC/method-param-named-id.m diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index e69fa152481982..ddf59bd748f343 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -1454,7 +1454,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, SmallVector KeyIdents; SmallVector KeyLocs; - SmallVector ObjCParamInfo; + SmallVector ArgInfos; ParseScope PrototypeScope(this, Scope::FunctionPrototypeScope | Scope::FunctionDeclarationScope | Scope::DeclScope); @@ -1495,9 +1495,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, ArgInfo.NameLoc = Tok.getLocation(); ConsumeToken(); // Eat the identifier. -ParmVarDecl *Param = Actions.ObjC().ActOnMethodParmDeclaration( -getCurScope(), ArgInfo, ObjCParamInfo.size(), MethodDefinition); -ObjCParamInfo.push_back(Param); +ArgInfos.push_back(ArgInfo); KeyIdents.push_back(SelIdent); KeyLocs.push_back(selLoc); @@ -1557,6 +1555,17 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc, nullptr)); } + // Turn ArgInfos into parameters. This must happen after parsing all + // parameters for bug compatibility with previous versions of Clang. (For + // instance, if a method declares a parameter called "id", that parameter must + // not shadow the "id" type.) + SmallVector ObjCParamInfo; + for (auto &ArgInfo : ArgInfos) { +ParmVarDecl *Param = Actions.ObjC().ActOnMethodParmDeclaration( +getCurScope(), ArgInfo, ObjCParamInfo.size(), MethodDefinition); +ObjCParamInfo.push_back(Param); + } + // FIXME: Add support for optional parameter list... // If attributes exist after the method, parse them. MaybeParseAttributes(PAKM_CXX11 | (getLangOpts().ObjC ? PAKM_GNU : 0), diff --git a/clang/test/SemaObjC/method-param-named-id.m b/clang/test/SemaObjC/method-param-named-id.m new file mode 100644 index 00..8269c31116c32d --- /dev/null +++ b/clang/test/SemaObjC/method-param-named-id.m @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s + + +@interface Foo +-(void)paramNamedID:(int)id usesIDType:(id)notShadowed; +-(void)paramNamedID:(int)id, id notShadowed; // expected-warning{{use of C-style parameters in Objective-C method declarations is deprecated}} +@end ___ cfe-commits mailing list cfe-commits@lists.llvm