[clang] [ObjC] Name lookup in methods shouldn't allow shadowing types (PR #116683)

[via cfe-commits]

https://github.com/apple-fcloutier created https://github.com/llvm/llvm-project/pull/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

>From ff4a663a8619b57836af10fbb321f8f42a1317eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Cloutier?= <fcloutier at apple.com>
Date: Mon, 18 Nov 2024 10:31:56 -0800
Subject: [PATCH] [ObjC] Name lookup in methods shouldn't allow shadowing types

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.

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
---
 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 239b70698e487c..bcbf4dfbabafa8 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -1454,7 +1454,7 @@ Decl *Parser::ParseObjCMethodDecl(SourceLocation mLoc,
 
   SmallVector<const IdentifierInfo *, 12> KeyIdents;
   SmallVector<SourceLocation, 12> KeyLocs;
-  SmallVector<ParmVarDecl *, 12> ObjCParamInfo;
+  SmallVector<SemaObjC::ObjCArgInfo, 12> 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<ParmVarDecl *, 12> 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 00000000000000..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
+
+
+ at 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}}
+ at end