[clang][ObjectiveC] Fix Parsing the :: Optional Scope Specifier

[qiongsiwu]

The parser hangs when processing types/variables prefixed by :: as an optional scope specifier. For example,

- (instancetype)init:(::A *) foo;

The parser should not hang, and it should emit an error. This PR implements the error check.

rdar://140885078

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qiongsi Wu (qiongsiwu)

Changes

The parser hangs when processing method parameters with types prefixed by ::. For example,

- (instancetype)init:(::A *) foo;

The parser should not hang, and it should emit an error. This PR implements the error check.

---

Full diff: https://github.com/llvm/llvm-project/pull/119908.diff

2 Files Affected:

• (modified) clang/lib/Parse/Parser.cpp (+7-1)
• (added) clang/test/Parser/objc-coloncolon.m (+5)

diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 36e56a92c3092e..aa78d702553172 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2222,8 +2222,14 @@ bool Parser::TryAnnotateTypeOrScopeTokenAfterScopeSpec(
     }
   }
 
-  if (SS.isEmpty())
+  if (SS.isEmpty()) {
+    if (getLangOpts().ObjC && Tok.is(tok::coloncolon)) {
+      // ObjectiveC does not allow :: as as a scope token.
+      Diag(ConsumeToken(), diag::err_expected_type);
+      return true;
+    }
     return false;
+  }
 
   // A C++ scope specifier that isn't followed by a typename.
   AnnotateScopeToken(SS, IsNewScope);
diff --git a/clang/test/Parser/objc-coloncolon.m b/clang/test/Parser/objc-coloncolon.m
new file mode 100644
index 00000000000000..e8a09898263bb3
--- /dev/null
+++ b/clang/test/Parser/objc-coloncolon.m
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -x objective-c -fsyntax-only -verify %s
+
+@interface A
+- (instancetype)init:(::A *) foo; // expected-error {{expected a type}} 
+@end

[qiongsiwu]

This fix tripped some tests in libc++. I am investigating.

[shafik]

This should have a release note.

[vsapsai]

How does it work in Objective-C++? I don't know even if we have a test but hope we do.

[cor3ntin]

How does it work in Objective-C++? I don't know even if we have a test but hope we do.

Right, this needs tests for Objective-C++

[vsapsai]

I don't know if we have a test for it but I've realized there are cases where you can have a legitimate double colon in Objective-C. For example,

@interface NSObject
@end

@implementation NSObject
- (void)performSelector:(SEL)selector {}

- (void)double:(int)firstArg :(int)secondArg colon:(int)thirdArg {}

- (void)test {
  [self performSelector:@selector(double::colon:)];
}
@end

It's not a method parameter type, so it is possible your code isn't executed. But it is worth checking if we test this case.

[qiongsiwu]

Thanks for the feedback everyone! The PR is updated to address the review comments.

• A test is added to cover Objective-C++. https://github.com/llvm/llvm-project/pull/119908/files#diff-7f1cce2d0397139fc73d3615c8249ac9530082aa707868c0e921f53daf594384R1 I am not sure if this is sufficient so please provide feedback.
• The Objective-C test is revised to cover the optional scope specifier on variables (in addition to parameters) https://github.com/llvm/llvm-project/pull/119908/files#diff-0c0e8844905a75c581903811a694e614f49b2945e2254a76bb5a215443028554R11.
• The release note is updated https://github.com/llvm/llvm-project/pull/119908/files#diff-ec770381d76c859f5f572db789175fe44410a72608f58ad5dbb14335ba56eb97R678.

[qiongsiwu]

The Objective-C test is modified to contain valid code with :: to include @vsapsai 's suggestion . https://github.com/llvm/llvm-project/pull/119908/files#diff-0c0e8844905a75c581903811a694e614f49b2945e2254a76bb5a215443028554R11

[qiongsiwu]

Gentle ping for review. Thanks so much!

[MaskRay]

(https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it/74223 the convention is to disable github's hide-email feature)