Skip to content

Conversation

@perry-ca
Copy link
Contributor

Changed the function arguments to take const Twine& instead of const char*. This will avoid converting StringRef's to C strings too soon (or ever).

@perry-ca perry-ca requested a review from abhina-sree October 29, 2025 14:01
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" llvm:support labels Oct 29, 2025
@perry-ca perry-ca self-assigned this Oct 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 29, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-support

Author: Sean Perry (perry-ca)

Changes

Changed the function arguments to take const Twine& instead of const char*. This will avoid converting StringRef's to C strings too soon (or ever).


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

4 Files Affected:

  • (modified) clang/lib/Basic/SourceManager.cpp (+1-2)
  • (modified) llvm/include/llvm/Support/AutoConvert.h (+4-3)
  • (modified) llvm/lib/Support/AutoConvert.cpp (+3-3)
  • (modified) llvm/lib/Support/MemoryBuffer.cpp (+1-1)
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index d8ec837f0f7b9..938c6485125ee 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -608,8 +608,7 @@ FileID SourceManager::createFileIDImpl(ContentCache &File, StringRef Filename,
     return FileID::get(LoadedID);
   }
   unsigned FileSize = File.getSize();
-  llvm::ErrorOr<bool> NeedConversion =
-      llvm::needConversion(Filename.str().c_str());
+  llvm::ErrorOr<bool> NeedConversion = llvm::needConversion(Filename);
   if (NeedConversion && *NeedConversion) {
     // Buffer size may increase due to potential z/OS EBCDIC to UTF-8
     // conversion.
diff --git a/llvm/include/llvm/Support/AutoConvert.h b/llvm/include/llvm/Support/AutoConvert.h
index 1e6792636e169..15f1ec8af6c57 100644
--- a/llvm/include/llvm/Support/AutoConvert.h
+++ b/llvm/include/llvm/Support/AutoConvert.h
@@ -18,6 +18,7 @@
 #include <_Ccsid.h>
 #endif
 #ifdef __cplusplus
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
 #include <system_error>
 #endif /* __cplusplus */
@@ -47,12 +48,12 @@ namespace llvm {
 std::error_code setzOSFileTag(int FD, int CCSID, bool Text);
 
 /** \brief Get the the tag ccsid for a file name or a file descriptor. */
-ErrorOr<__ccsid_t> getzOSFileTag(const char *FileName, const int FD = -1);
+ErrorOr<__ccsid_t> getzOSFileTag(const Twine &FileName, const int FD = -1);
 
 /** \brief Query the file tag to determine if it needs conversion to UTF-8
  *  codepage.
  */
-ErrorOr<bool> needzOSConversion(const char *FileName, const int FD = -1);
+ErrorOr<bool> needzOSConversion(const Twine &FileName, const int FD = -1);
 
 #endif /* __MVS__*/
 
@@ -87,7 +88,7 @@ inline std::error_code setFileTag(int FD, int CCSID, bool Text) {
   return std::error_code();
 }
 
-inline ErrorOr<bool> needConversion(const char *FileName, const int FD = -1) {
+inline ErrorOr<bool> needConversion(const Twine &FileName, const int FD = -1) {
 #ifdef __MVS__
   return needzOSConversion(FileName, FD);
 #endif
diff --git a/llvm/lib/Support/AutoConvert.cpp b/llvm/lib/Support/AutoConvert.cpp
index 0b6928e10ef5a..741bb7bd2c5b0 100644
--- a/llvm/lib/Support/AutoConvert.cpp
+++ b/llvm/lib/Support/AutoConvert.cpp
@@ -96,7 +96,7 @@ std::error_code llvm::setzOSFileTag(int FD, int CCSID, bool Text) {
   return std::error_code();
 }
 
-ErrorOr<__ccsid_t> llvm::getzOSFileTag(const char *FileName, const int FD) {
+ErrorOr<__ccsid_t> llvm::getzOSFileTag(const Twine &FileName, const int FD) {
   // If we have a file descriptor, use it to find out file tagging. Otherwise we
   // need to use stat() with the file path.
   if (FD != -1) {
@@ -110,12 +110,12 @@ ErrorOr<__ccsid_t> llvm::getzOSFileTag(const char *FileName, const int FD) {
     return Query.fccsid;
   }
   struct stat Attr;
-  if (stat(FileName, &Attr) == -1)
+  if (stat(FileName.str().c_str(), &Attr) == -1)
     return std::error_code(errno, std::generic_category());
   return Attr.st_tag.ft_ccsid;
 }
 
-ErrorOr<bool> llvm::needzOSConversion(const char *FileName, const int FD) {
+ErrorOr<bool> llvm::needzOSConversion(const Twine &FileName, const int FD) {
   ErrorOr<__ccsid_t> Ccsid = getzOSFileTag(FileName, FD);
   if (std::error_code EC = Ccsid.getError())
     return EC;
diff --git a/llvm/lib/Support/MemoryBuffer.cpp b/llvm/lib/Support/MemoryBuffer.cpp
index 1c4645ad83641..23b9f8c5790d2 100644
--- a/llvm/lib/Support/MemoryBuffer.cpp
+++ b/llvm/lib/Support/MemoryBuffer.cpp
@@ -512,7 +512,7 @@ getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
   }
 
 #ifdef __MVS__
-  ErrorOr<bool> NeedsConversion = needConversion(Filename.str().c_str(), FD);
+  ErrorOr<bool> NeedsConversion = needConversion(Filename, FD);
   if (std::error_code EC = NeedsConversion.getError())
     return EC;
   // File size may increase due to EBCDIC -> UTF-8 conversion, therefore we

Copy link
Contributor

@abhina-sree abhina-sree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@perry-ca perry-ca merged commit 3d2efe7 into llvm:main Oct 29, 2025
14 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 29, 2025

LLVM Buildbot has detected a new failure on builder openmp-s390x-linux running on systemz-1 while building clang,llvm at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/17631

Here is the relevant piece of the build log for the reference
Step 6 (test-openmp) failure: 1200 seconds without output running [b'ninja', b'-j 4', b'check-openmp'], attempting to kill
...
PASS: ompd-test :: openmp_examples/example_3.c (452 of 462)
PASS: ompd-test :: openmp_examples/example_4.c (453 of 462)
PASS: ompd-test :: openmp_examples/example_5.c (454 of 462)
PASS: ompd-test :: openmp_examples/example_task.c (455 of 462)
UNSUPPORTED: ompd-test :: openmp_examples/ompd_bt.c (456 of 462)
PASS: ompd-test :: openmp_examples/fibonacci.c (457 of 462)
UNSUPPORTED: ompd-test :: openmp_examples/ompd_parallel.c (458 of 462)
PASS: ompd-test :: openmp_examples/parallel.c (459 of 462)
PASS: ompd-test :: openmp_examples/nested.c (460 of 462)
PASS: ompd-test :: openmp_examples/ompd_icvs.c (461 of 462)
command timed out: 1200 seconds without output running [b'ninja', b'-j 4', b'check-openmp'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1375.690396

aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
Changed the function arguments to take `const Twine&` instead of `const
char*`. This will avoid converting StringRef's to C strings too soon (or
ever).
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
Changed the function arguments to take `const Twine&` instead of `const
char*`. This will avoid converting StringRef's to C strings too soon (or
ever).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants