-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Fix MSVC warning in CompilerInvocation.cpp #152809
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
Conversation
|
@llvm/pr-subscribers-clang Author: Daniel Paoliello (dpaoliello) ChangesBuilding Clang using MSVC was resulting in the following warning: I traced this to CompilerInvocation.cpp where it was creating a This change adds an explicit type for the Full diff: https://github.com/llvm/llvm-project/pull/152809.diff 1 Files Affected:
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index ccc3154d20968..d9260e12cec3f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4454,7 +4454,7 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
: VerParts.first.size() == Ver.size() || VerParts.second == "0")) {
// Got a valid version number.
#define ABI_VER_MAJOR_MINOR(Major_, Minor_) \
- if (std::tie(Major, Minor) <= std::tuple(Major_, Minor_)) \
+ if (std::tuple(Major, Minor) <= std::tuple<unsigned, unsigned>(Major_, Minor_)) \
Opts.setClangABICompat(LangOptions::ClangABI::Ver##Major_##_##Minor_); \
else
#define ABI_VER_MAJOR(Major_) \
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
Thanks!
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.
Can we simply change unsigned to int in Line 4444? What do you think?
int Major, Minor = 0;
Yep, that works. |
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
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/14920 Here is the relevant piece of the build log for the reference |
Building Clang using MSVC was resulting in the following warning:
I traced this to CompilerInvocation.cpp where it was creating a
std::tupleto compare version numbers.This change adds an explicit type for the
tuplecreated from the version macros to match the type of the variables, and uses thetupleconstructor instead oftiesince the integers are smaller than a reference to the integers.