Skip to content

Conversation

sharadhr
Copy link
Contributor

@sharadhr sharadhr commented Aug 8, 2024

Manual PR to backport #99300.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Aug 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2024

@llvm/pr-subscribers-clang-driver

Author: Sharadh Rajaraman (sharadhr)

Changes

Manual PR to backport #99300.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Types.cpp (+3-1)
  • (added) clang/test/Driver/cl-cxx20-modules.cppm (+16)
diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index a7b6b9000e1d2..2b9b391c19c9f 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -242,7 +242,9 @@ bool types::isCXX(ID Id) {
   case TY_CXXHUHeader:
   case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
-  case TY_CXXModule: case TY_PP_CXXModule:
+  case TY_CXXModule:
+  case TY_PP_CXXModule:
+  case TY_ModuleFile:
   case TY_PP_CLCXX:
   case TY_CUDA: case TY_PP_CUDA: case TY_CUDA_DEVICE:
   case TY_HIP:
diff --git a/clang/test/Driver/cl-cxx20-modules.cppm b/clang/test/Driver/cl-cxx20-modules.cppm
new file mode 100644
index 0000000000000..43dbf517485a0
--- /dev/null
+++ b/clang/test/Driver/cl-cxx20-modules.cppm
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cl /std:c++20 --precompile -### -- %s 2>&1 | FileCheck --check-prefix=PRECOMPILE %s
+// PRECOMPILE: -emit-module-interface
+
+// RUN: %clang_cl /std:c++20 --fmodule-file=Foo=Foo.pcm -### -- %s 2>&1 | FileCheck --check-prefix=FMODULEFILE %s
+// FMODULEFILE: -fmodule-file=Foo=Foo.pcm
+
+// RUN: %clang_cl /std:c++20 --fprebuilt-module-path=. -### -- %s 2>&1 | FileCheck --check-prefix=FPREBUILT %s
+// FPREBUILT: -fprebuilt-module-path=.
+
+// RUN: %clang_cl %t/test.pcm /std:c++20 -### 2>&1 | FileCheck --check-prefix=CPP20WARNING %t/test.pcm
+
+//--- test.pcm
+// CPP20WARNING-NOT: clang-cl: warning: argument unused during compilation: '/std:c++20' [-Wunused-command-line-argument]

@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2024

@llvm/pr-subscribers-clang

Author: Sharadh Rajaraman (sharadhr)

Changes

Manual PR to backport #99300.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Types.cpp (+3-1)
  • (added) clang/test/Driver/cl-cxx20-modules.cppm (+16)
diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index a7b6b9000e1d2..2b9b391c19c9f 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -242,7 +242,9 @@ bool types::isCXX(ID Id) {
   case TY_CXXHUHeader:
   case TY_PP_CXXHeaderUnit:
   case TY_ObjCXXHeader: case TY_PP_ObjCXXHeader:
-  case TY_CXXModule: case TY_PP_CXXModule:
+  case TY_CXXModule:
+  case TY_PP_CXXModule:
+  case TY_ModuleFile:
   case TY_PP_CLCXX:
   case TY_CUDA: case TY_PP_CUDA: case TY_CUDA_DEVICE:
   case TY_HIP:
diff --git a/clang/test/Driver/cl-cxx20-modules.cppm b/clang/test/Driver/cl-cxx20-modules.cppm
new file mode 100644
index 0000000000000..43dbf517485a0
--- /dev/null
+++ b/clang/test/Driver/cl-cxx20-modules.cppm
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cl /std:c++20 --precompile -### -- %s 2>&1 | FileCheck --check-prefix=PRECOMPILE %s
+// PRECOMPILE: -emit-module-interface
+
+// RUN: %clang_cl /std:c++20 --fmodule-file=Foo=Foo.pcm -### -- %s 2>&1 | FileCheck --check-prefix=FMODULEFILE %s
+// FMODULEFILE: -fmodule-file=Foo=Foo.pcm
+
+// RUN: %clang_cl /std:c++20 --fprebuilt-module-path=. -### -- %s 2>&1 | FileCheck --check-prefix=FPREBUILT %s
+// FPREBUILT: -fprebuilt-module-path=.
+
+// RUN: %clang_cl %t/test.pcm /std:c++20 -### 2>&1 | FileCheck --check-prefix=CPP20WARNING %t/test.pcm
+
+//--- test.pcm
+// CPP20WARNING-NOT: clang-cl: warning: argument unused during compilation: '/std:c++20' [-Wunused-command-line-argument]

@sharadhr
Copy link
Contributor Author

@ChuanqiXu9, is there anything else that needs to be done here? There's a merge conflict; I could resolve that.

@ChuanqiXu9 ChuanqiXu9 added this to the LLVM 19.X Release milestone Aug 15, 2024
@ChuanqiXu9
Copy link
Member

@ChuanqiXu9, is there anything else that needs to be done here? There's a merge conflict; I could resolve that.

If there is merge conflict, we need to resolve it. For the merge request, we need to wait for the release manager to have a time to look at this. Maybe due to we forgot to mention this belongs to 19.x. CC @tru manually.

@tru
Copy link
Collaborator

tru commented Aug 19, 2024

This PR is pretty messy: Several commits instead of a single that can be cherry-picked, merge commit that makes it harder to cherry-pick and squash.

Please update the PR to contain a single commit that fixes the issue and doesn't contain any merge commits, then I can merge it easily.

Thanks.

@sharadhr sharadhr force-pushed the clang-cl-modules-unused-argument branch from c7b53d3 to 909706c Compare August 19, 2024 11:18
@sharadhr
Copy link
Contributor Author

@tru, thanks for the feedback; I hope this is better.

@tru
Copy link
Collaborator

tru commented Aug 19, 2024

Looks much better. I'll merge it later.

@tru tru force-pushed the clang-cl-modules-unused-argument branch from 909706c to 6fcbfb8 Compare August 20, 2024 07:15
@tru tru merged commit 6fcbfb8 into llvm:release/19.x Aug 20, 2024
8 of 9 checks passed
Copy link

@sharadhr (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@sharadhr sharadhr deleted the clang-cl-modules-unused-argument branch August 15, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants