[clang] Add getClangVendor() and use it in CodeGenModule.cpp

[DimitryAndric]

In 9a38a72 ProductId was assigned from the stringified value of CLANG_VENDOR, if that macro was defined. However, CLANG_VENDOR is supposed to be a string, as it is defined (optionally) as such in the top-level clang CMakeLists.txt.

Move the addition of -DCLANG_VENDOR to the compiler flags from clang/lib/Basic/CMakeLists.txt to the top-level CMakeLists.txt, so it is consistent across the whole clang codebase. Then remove the stringification from CodeGenModule.cpp, to make it work correctly.

Fixes: 9a38a72

[llvmbot]

@llvm/pr-subscribers-clang-codegen

Author: Dimitry Andric (DimitryAndric)

Changes

In 9a38a72 ProductId was assigned from the stringified value of CLANG_VENDOR, if that macro was defined. However, CLANG_VENDOR is supposed to be a string, as it is defined (optionally) as such in the top-level clang CMakeLists.txt.

Move the addition of -DCLANG_VENDOR to the compiler flags from clang/lib/Basic/CMakeLists.txt to the top-level CMakeLists.txt, so it is consistent across the whole clang codebase. Then remove the stringification from CodeGenModule.cpp, to make it work correctly.

Fixes: 9a38a72

---

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

3 Files Affected:

• (modified) clang/CMakeLists.txt (+4)
• (modified) clang/lib/Basic/CMakeLists.txt (-5)
• (modified) clang/lib/CodeGen/CodeGenModule.cpp (+1-1)

diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 2ca6db02e58791..c7461bc510e4ac 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -241,6 +241,10 @@ set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING "SystemZ Default Arch")
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
+if(CLANG_VENDOR)
+  add_definitions(-DCLANG_VENDOR="${CLANG_VENDOR}")
+endif()
+
 set(CLANG_REPOSITORY_STRING "" CACHE STRING
   "Vendor-specific text for showing the repository the source is taken from.")
 
diff --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index 2e218ba7c84cca..6b6fde9b5a9a6b 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -49,11 +49,6 @@ set_source_files_properties("${version_inc}"
   PROPERTIES GENERATED TRUE
              HEADER_FILE_ONLY TRUE)
 
-if(CLANG_VENDOR)
-  set_source_files_properties(Version.cpp
-    PROPERTIES COMPILE_DEFINITIONS "CLANG_VENDOR=\"${CLANG_VENDOR} \"")
-endif()
-
 add_clang_library(clangBasic
   Attributes.cpp
   Builtins.cpp
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 7ad26ace328ab2..c8007aef675cb4 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -997,7 +997,7 @@ void CodeGenModule::Release() {
                               uint32_t(CLANG_VERSION_PATCHLEVEL));
     std::string ProductId;
 #ifdef CLANG_VENDOR
-    ProductId = #CLANG_VENDOR;
+    ProductId = CLANG_VENDOR;
 #else
     ProductId = "clang";
 #endif

[llvmbot]

@llvm/pr-subscribers-clang

Author: Dimitry Andric (DimitryAndric)

Changes

In 9a38a72 ProductId was assigned from the stringified value of CLANG_VENDOR, if that macro was defined. However, CLANG_VENDOR is supposed to be a string, as it is defined (optionally) as such in the top-level clang CMakeLists.txt.

Move the addition of -DCLANG_VENDOR to the compiler flags from clang/lib/Basic/CMakeLists.txt to the top-level CMakeLists.txt, so it is consistent across the whole clang codebase. Then remove the stringification from CodeGenModule.cpp, to make it work correctly.

Fixes: 9a38a72

---

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

3 Files Affected:

• (modified) clang/CMakeLists.txt (+4)
• (modified) clang/lib/Basic/CMakeLists.txt (-5)
• (modified) clang/lib/CodeGen/CodeGenModule.cpp (+1-1)

diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 2ca6db02e58791..c7461bc510e4ac 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -241,6 +241,10 @@ set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING "SystemZ Default Arch")
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
+if(CLANG_VENDOR)
+  add_definitions(-DCLANG_VENDOR="${CLANG_VENDOR}")
+endif()
+
 set(CLANG_REPOSITORY_STRING "" CACHE STRING
   "Vendor-specific text for showing the repository the source is taken from.")
 
diff --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index 2e218ba7c84cca..6b6fde9b5a9a6b 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -49,11 +49,6 @@ set_source_files_properties("${version_inc}"
   PROPERTIES GENERATED TRUE
              HEADER_FILE_ONLY TRUE)
 
-if(CLANG_VENDOR)
-  set_source_files_properties(Version.cpp
-    PROPERTIES COMPILE_DEFINITIONS "CLANG_VENDOR=\"${CLANG_VENDOR} \"")
-endif()
-
 add_clang_library(clangBasic
   Attributes.cpp
   Builtins.cpp
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 7ad26ace328ab2..c8007aef675cb4 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -997,7 +997,7 @@ void CodeGenModule::Release() {
                               uint32_t(CLANG_VERSION_PATCHLEVEL));
     std::string ProductId;
 #ifdef CLANG_VENDOR
-    ProductId = #CLANG_VENDOR;
+    ProductId = CLANG_VENDOR;
 #else
     ProductId = "clang";
 #endif

[smeenai]

IIRC, applying definitions like this only to a small set of files was an intentional decision, to minimize rebuilding when their values changed. It matters much more for the commit hash than the vendor, but passing defines like this to every single compilation in Clang is ugly IMO. Can we instead add a function to Version.cpp to access the vendor (so that the correct type gets encoded in the function signature) and use that from CodeGenModule.cpp instead of referencing CLANG_VENDOR directly?

[ysyeda]

LGTM, thanks

[petrhosek]

I agree with @smeenai, we shouldn't be setting CLANG_VENDOR as a global define.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[DimitryAndric]

Note that this is the way you are supposed to get a "product-id-with-vendor", as the vendor tag itself normally does not contain the word "clang". For example, FreeBSD uses "FreeBSD " (so with a trailing space), and Apple uses "Apple " (also with trailing space). If any space is desired between the vendor tag and the word "clang", it has to be specified in the vendor tag, and not appended here in the string concatenation.

[smeenai]

LGTM, thanks!