From a5c1dacdf7bc2cd0434dc11c17033b162a494bb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix=20Cloutier?= <fcloutier@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
(cherry picked from commit d8c7c55237019791ec3badea535ca227bd22845a)
---
 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 45929e6a48d8a..86210ab7bac2b 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -1455,7 +1455,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);
 
@@ -1496,9 +1496,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);
 
@@ -1558,6 +1556,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 0000000000000..8269c31116c32
--- /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