-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[clang][driver][clang-cl] Fix unused argument warning for /std:c++20
for precompiled module inputs to clang-cl
#99300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang][driver][clang-cl] Fix unused argument warning for /std:c++20
for precompiled module inputs to clang-cl
#99300
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Sharadh Rajaraman (sharadhr) ChangesRelates to #98761: Full diff: https://github.com/llvm/llvm-project/pull/99300.diff 1 Files Affected:
diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index a7b6b9000e1d2..842eb59b7382d 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:
|
/std:c++20
for precompiled module inputs to clang-cl
/std:c++20
for precompiled module inputs to clang-cl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test for this too. We can use any file ends with .pcm
to mimic module file in the Driver's test and we can use CHECK-NOT:
to filter the warning.
fe0e22a
to
cbe5a79
Compare
Not quite sure how to handle the test for this; I'm struggling to find a |
✅ With the latest revision this PR passed the C/C++ code formatter. |
cbe5a79
to
3b9c3d2
Compare
Maybe something like this:
|
f1d83d6
to
ad840c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@sharadhr Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
/cherry-pick 9d315bc |
Failed to cherry-pick: 9d315bc https://github.com/llvm/llvm-project/actions/runs/10294654014 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
Just to clarify—you'd like me to open another PR to In fact I'm a little more ambitious because this and #98761 are honestly very simple PRs—I'd like them to be backported as far as Clang 16 (which is when modules support first appeared) if possible. |
Yes. You can find the automated generated example in #102159
No, this is not possible. We may never have new releases for 18, 17 and 16 as far as I know. |
Ah. That's a pity but no problem. I'll create the PR now. Thanks! |
1. __cpp_lib_modules now is defined. llvm/llvm-project#90091 2. clang-cl supports -x option for all languages. llvm/llvm-project#89772 3. Header units allow non-inline external static members definitions. llvm/llvm-project#98309 4. winsysroot alias to the GNU driver: -Xmicrosoft-windows-sys-root. llvm/llvm-project#94731 5. clang-cl gained options for compiling C++ modules. llvm/llvm-project#98761 6. fix unused argument warning for /std:c++20 when compiling c++ modules. llvm/llvm-project#99300
Relates to #98761:
-fmodule-file
requires/std:c++20
or greater, but passing that option results in an unused argument warning. This resolves that.