Skip to content

Conversation

@tbaederr
Copy link
Contributor

@tbaederr tbaederr commented Sep 4, 2025

Otherwise we get the char representation in our disassembly output, which we don't want.

Otherwise we get the char representation in our disassembly output,
which we don't want.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Sep 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Otherwise we get the char representation in our disassembly output, which we don't want.


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

1 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Disasm.cpp (+14-1)
diff --git a/clang/lib/AST/ByteCode/Disasm.cpp b/clang/lib/AST/ByteCode/Disasm.cpp
index ac904d359d8cc..ab3b9f7c3b1d7 100644
--- a/clang/lib/AST/ByteCode/Disasm.cpp
+++ b/clang/lib/AST/ByteCode/Disasm.cpp
@@ -44,7 +44,20 @@ inline static std::string printArg(Program &P, CodePtr &OpPC) {
     std::string Result;
     llvm::raw_string_ostream SS(Result);
     auto Arg = OpPC.read<T>();
-    SS << Arg;
+    // Make sure we print the integral value of chars.
+    if constexpr (std::is_integral_v<T>) {
+      if constexpr (sizeof(T) == 1) {
+        if constexpr (std::is_signed_v<T>)
+          SS << static_cast<int32_t>(Arg);
+        else
+          SS << static_cast<uint32_t>(Arg);
+      } else {
+        SS << Arg;
+      }
+    } else {
+      SS << Arg;
+    }
+
     return Result;
   }
 }

@tbaederr tbaederr merged commit 8f37668 into llvm:main Sep 4, 2025
13 checks passed
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

I am a surprised there are not changed tests nor additional test given the problem description. It would seem we are avoiding output in a specific format.

We should have a test in which the output would differ w/o this change.

@tbaederr
Copy link
Contributor Author

tbaederr commented Sep 5, 2025

This only changes debugging output so there are no tests for it.

@shafik
Copy link
Collaborator

shafik commented Sep 5, 2025

This only changes debugging output so there are no tests for it.

So it looks like this output is only used for dump() functions, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants