[clang][deps] Don't treat ObjC method args as module directives

[Bigcheese]

import:(type)name is a method argument decl in ObjC, but the C++20 preprocessing rules say this is a preprocessing line.

Because the dependency directive scanner is not language dependent, this patch extends the C++20 rule to exclude module : (which is never a valid module decl anyway), and import : that is not followed by an identifier.

This is ok to do because in C++20 mode the compiler will later error on lines like this anyway, and the dependencies the scanner returns are still correct.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Michael Spencer (Bigcheese)

Changes

import:(type)name is a method argument decl in ObjC, but the C++20 preprocessing rules say this is a preprocessing line.

Because the dependency directive scanner is not language dependent, this patch extends the C++20 rule to exclude module : (which is never a valid module decl anyway), and import : that is not followed by an identifier.

This is ok to do because in C++20 mode the compiler will later error on lines like this anyway, and the dependencies the scanner returns are still correct.

---

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

2 Files Affected:

• (modified) clang/lib/Lex/DependencyDirectivesScanner.cpp (+12-1)
• (added) clang/test/ClangScanDeps/scanner-objc-compat.m (+28)

diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 0971daa1f3666..bd2a8d5115c81 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -660,7 +660,18 @@ bool Scanner::lexModule(const char *&First, const char *const End) {
   // an import.
 
   switch (*First) {
-  case ':':
+  case ':': {
+    // `module :` is never the start of a valid module declaration.
+    if (Id == "module") {
+      skipLine(First, End);
+      return false;
+    }
+    // `import:(type)name` is a valid ObjC method decl, so check one more token.
+    (void)lexToken(First, End);
+    if (!tryLexIdentifierOrSkipLine(First, End))
+      return false;
+    break;
+  }
   case '<':
   case '"':
     break;
diff --git a/clang/test/ClangScanDeps/scanner-objc-compat.m b/clang/test/ClangScanDeps/scanner-objc-compat.m
new file mode 100644
index 0000000000000..302988f1f51d6
--- /dev/null
+++ b/clang/test/ClangScanDeps/scanner-objc-compat.m
@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: clang-scan-deps -format experimental-full -- \
+// RUN:   %clang -c %t/main.m -o %t/main.o -fmodules -I %t > %t/deps.db
+// RUN: cat %t/deps.db | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// Verify that the scanner does not treat ObjC method decls with arguments named
+// module or import as module/import decls.
+
+// CHECK: "module-name": "A"
+
+//--- A.h
+
+@interface SomeObjcClass
+  - (void)func:(int)otherData
+          module:(int)module
+          import:(int)import;
+@end
+
+//--- module.modulemap
+
+module A {
+  header "A.h"
+}
+
+//--- main.m
+
+#include "A.h"

[ChuanqiXu9]

LGTM as it looks like not affecting C++20 modules.

[akyrtzi]

Could you test the change with a unit test in DependencyDirectivesScannerTest.cpp instead? It would be orders of magnitude more lightweight, execution wise, than a whole new lit test.

[Bigcheese]

Updated the test to be a unit test.