-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Don't use CLANG_VERSION_MAJOR to check that the value passed to -fclang-abi-compat= is valid #123998
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
-fclang-abi-compat= is valid Use LLVM_VERSION_MAJOR instead as the maximum allowed value. This change is needed to fix regression tests that fail when vendors set CLANG_VERSION_MAJOR to a value that is lower than LLVM_VERSION_MAJOR when building the compiler. For example, clang/test/CodeGenCXX/mangle-concept.cpp fails with the following error if -DCLANG_VERSION_MAJOR=17 is passed to cmake: invalid value '19' in '-fclang-abi-compat=19'
@llvm/pr-subscribers-clang Author: Akira Hatanaka (ahatanak) ChangesUse LLVM_VERSION_MAJOR instead as the maximum allowed value. This change is needed to fix regression tests that fail For example, clang/test/CodeGenCXX/mangle-concept.cpp fails with the following error if -DCLANG_VERSION_MAJOR=17 invalid value '19' in '-fclang-abi-compat=19' Full diff: https://github.com/llvm/llvm-project/pull/123998.diff 3 Files Affected:
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index b79e570667b2c4..4a18ecf8d2a757 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -331,6 +331,7 @@ if(NOT DEFINED CLANG_VERSION_SUFFIX)
set(CLANG_VERSION_SUFFIX ${LLVM_VERSION_SUFFIX})
endif()
set(CLANG_VERSION "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}.${CLANG_VERSION_PATCHLEVEL}${CLANG_VERSION_SUFFIX}")
+set(MAX_CLANG_ABI_COMPAT_VERSION "${LLVM_VERSION_MAJOR}")
message(STATUS "Clang version: ${CLANG_VERSION}")
# Configure the Version.inc file.
diff --git a/clang/include/clang/Basic/Version.inc.in b/clang/include/clang/Basic/Version.inc.in
index 370da8506d1425..69c79d03861cd2 100644
--- a/clang/include/clang/Basic/Version.inc.in
+++ b/clang/include/clang/Basic/Version.inc.in
@@ -4,3 +4,4 @@
#define CLANG_VERSION_MAJOR_STRING "@CLANG_VERSION_MAJOR@"
#define CLANG_VERSION_MINOR @CLANG_VERSION_MINOR@
#define CLANG_VERSION_PATCHLEVEL @CLANG_VERSION_PATCHLEVEL@
+#define MAX_CLANG_ABI_COMPAT_VERSION @MAX_CLANG_ABI_COMPAT_VERSION@
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index d711df02ce9503..09eb0517544a27 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4387,7 +4387,7 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
// y or y.0 (4 <= y <= current version).
if (!VerParts.first.starts_with("0") &&
!VerParts.first.getAsInteger(10, Major) && 3 <= Major &&
- Major <= CLANG_VERSION_MAJOR &&
+ Major <= MAX_CLANG_ABI_COMPAT_VERSION &&
(Major == 3
? VerParts.second.size() == 1 &&
!VerParts.second.getAsInteger(10, Minor)
|
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/153/builds/21266 Here is the relevant piece of the build log for the reference
|
Use LLVM_VERSION_MAJOR instead as the maximum allowed value. This change is needed to fix regression tests that fail
when vendors set CLANG_VERSION_MAJOR to a value that is lower than LLVM_VERSION_MAJOR when building the
compiler.
For example, clang/test/CodeGenCXX/mangle-concept.cpp fails with the following error if -DCLANG_VERSION_MAJOR=17
is passed to cmake:
invalid value '19' in '-fclang-abi-compat=19'