[clang] [clang][deps] Don't treat ObjC method args as module directives (PR #97654)

Michael Spencer via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 11 16:32:27 PDT 2024


https://github.com/Bigcheese updated https://github.com/llvm/llvm-project/pull/97654

>From 1e8ecb546c3519488050a2c1abe22bb0ecbb8495 Mon Sep 17 00:00:00 2001
From: Michael Spencer <bigcheesegs at gmail.com>
Date: Wed, 10 Jul 2024 17:02:39 -0700
Subject: [PATCH] [clang][deps] Don't treat ObjC method args as module
 directives

`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.
---
 clang/lib/Lex/DependencyDirectivesScanner.cpp | 13 +++++++++++-
 .../Lex/DependencyDirectivesScannerTest.cpp   | 20 +++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 57652be8244b4..8a020d0e95fe3 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/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 94af9688a96e2..96b8cbc3888f5 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -970,6 +970,26 @@ ort \
   EXPECT_EQ(Directives[1].Kind, cxx_export_module_decl);
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, ObjCMethodArgs) {
+  SmallVector<char, 128> Out;
+  SmallVector<dependency_directives_scan::Token, 4> Tokens;
+  SmallVector<Directive, 4> Directives;
+
+  StringRef Source = R"(
+    @interface SomeObjcClass
+      - (void)func:(int)otherData
+              module:(int)module
+              import:(int)import;
+    @end
+  )";
+
+  ASSERT_FALSE(
+      minimizeSourceToDependencyDirectives(Source, Out, Tokens, Directives));
+  // `module :` and `import :` not followed by an identifier are not treated as
+  // directive lines because they can be method argument decls.
+  EXPECT_EQ(Directives.size(), 2u); // 2 for the EOF tokens.
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, TokensBeforeEOF) {
   SmallString<128> Out;
 



More information about the cfe-commits mailing list