[clang-repl] Fix struct value printing for clang-repl in C mode #165538
+12
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Now we have |
llvmbot
added
clang
anutosh491
changed the title
|
@llvm/pr-subscribers-clang Author: Anutosh Bhat (anutosh491) ChangesI added some logs to see the difference between C++ mode and C mode and I see this In C++ mode in C mode So basically C mode wasn't entering block 540 as expressions like Full diff: https://github.com/llvm/llvm-project/pull/165538.diff 2 Files Affected:
diff --git a/clang/lib/Interpreter/InterpreterValuePrinter.cpp b/clang/lib/Interpreter/InterpreterValuePrinter.cpp
index 0ed02f3bfabe8..1e9777f3485c6 100644
--- a/clang/lib/Interpreter/InterpreterValuePrinter.cpp
+++ b/clang/lib/Interpreter/InterpreterValuePrinter.cpp
@@ -411,7 +411,7 @@ class InterfaceKindVisitor
}
InterfaceKind VisitReferenceType(const ReferenceType *Ty) {
- ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, E);
+ ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, E->IgnoreImpCasts());
assert(!AddrOfE.isInvalid() && "Can not create unary expression");
Args.push_back(AddrOfE.get());
return InterfaceKind::NoAlloc;
@@ -537,7 +537,7 @@ llvm::Expected<Expr *> Interpreter::convertExprToValue(Expr *E) {
QualType DesugaredTy = Ty.getDesugaredType(Ctx);
// For lvalue struct, we treat it as a reference.
- if (DesugaredTy->isRecordType() && E->isLValue()) {
+ if (DesugaredTy->isRecordType() && E->IgnoreImpCasts()->isLValue()) {
DesugaredTy = Ctx.getLValueReferenceType(DesugaredTy);
Ty = Ctx.getLValueReferenceType(Ty);
}
diff --git a/clang/test/Interpreter/pretty-print.c b/clang/test/Interpreter/pretty-print.c
index d0712fb152107..c752b53b5264a 100644
--- a/clang/test/Interpreter/pretty-print.c
+++ b/clang/test/Interpreter/pretty-print.c
@@ -78,14 +78,16 @@ int * null_ptr = (int*)0; null_ptr
union U { int I; float F; } u; u.I = 12; u.I
// CHECK-NEXT: (int) 12
-// TODO: _Bool, _Complex, _Atomic, and _BitInt
-// struct S1{} s1; s1
-// TODO-CHECK-NEXT: (S1 &) @0x{{[0-9a-f]+}}
+struct S1{} s1; s1
+// CHECK-NEXT: (S1 &) @0x{{[0-9a-f]+}}
+
+struct S2 {int d;} E = {22}; E
+// CHECK-NEXT: (struct S2 &) @0x{{[0-9a-f]+}}
-// struct S2 {int d;} E = {22}; E
-// TODO-CHECK-NEXT: (struct S2 &) @0x{{[0-9a-f]+}}
-// E.d
-// TODO-CHECK-NEXT: (int) 22
+E.d
+// CHECK-NEXT: (int) 22
+
+// TODO: _Bool, _Complex, _Atomic, and _BitInt
// -----------------------------------------------------------------------------
// Tentative definition handling (C99 6.9.2)
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Thanks for the review. Merging ! |
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
…#165538) I added some logs to see the difference between C++ mode and C mode and I see this In C++ mode ``` clang-repl> struct S1{} s1; s1 [convertExprToValue] original Expr: DeclRefExpr | type: struct S1 [convertExprToValue] Ty: struct S1 [convertExprToValue] DesugaredTy: struct S1 [convertExprToValue] Treating lvalue record as reference (enters block 540) [convertExprToValue] Ty: struct S1 & (after block 540) [convertExprToValue] DesugaredTy: struct S1 & (after block 540) [computeInterfaceKind] Expr class: DeclRefExpr | isLValue: 1 (S1 &) @0x10c9ac058 ``` in C mode ``` (base) anutosh491@Anutoshs-MacBook-Air bin % ./clang-repl --Xcc=-xc --Xcc=-std=c23 clang-repl> struct S1{} s1; s1 [convertExprToValue] original Expr: ImplicitCastExpr | type: struct S1 [convertExprToValue] Ty: struct S1 [convertExprToValue] DesugaredTy: struct S1 [convertExprToValue] Ty: struct S1 (after block 540) [convertExprToValue] DesugaredTy: struct S1 (after block 540) [computeInterfaceKind] Expr class: ImplicitCastExpr | isLValue: 0 Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): s0 clang-repl 0x0000000103cca03c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 88 1 clang-repl 0x0000000103cca61c PrintStackTraceSignalHandler(void*) + 28 2 clang-repl 0x0000000103cc7ee8 llvm::sys::RunSignalHandlers() + 152 3 clang-repl 0x0000000103ccbb54 SignalHandler(int, __siginfo*, void*) + 284 4 libsystem_platform.dylib 0x00000001887f4624 _sigtramp + 56 5 clang-repl 0x00000001079bee18 clang::Sema::CheckArgsForPlaceholders(llvm::MutableArrayRef<clang::Expr*>) + 120 6 clang-repl 0x00000001079bee18 clang::Sema::CheckArgsForPlaceholders(llvm::MutableArrayRef<clang::Expr*>) + 120 7 clang-repl 0x0000000107b823dc clang::Sema::BuildCXXNew(clang::SourceRange, bool, clang::SourceLocation, llvm::MutableArrayRef<clang::Expr*>, clang::SourceLocation, clang::SourceRange, clang::QualType, clang::TypeSourceInfo*, std::__1::optional<clang::Expr*>, clang::SourceRange, clang::Expr*) + 5672 8 clang-repl 0x000000010538c560 clang::Interpreter::convertExprToValue(clang::Expr*) + 2580 9 clang-repl 0x0000000105360774 clang::InProcessPrintingASTConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 252 10 clang-repl 0x000000010536a82c clang::IncrementalParser::ParseOrWrapTopLevelDecl() + 676 11 clang-repl 0x000000010536b554 clang::IncrementalParser::Parse(llvm::StringRef) + 712 12 clang-repl 0x000000010537e6b4 clang::Interpreter::Parse(llvm::StringRef) + 588 13 clang-repl 0x000000010537d73c clang::Interpreter::ParseAndExecute(llvm::StringRef, clang::Value*) + 72 14 clang-repl 0x000000010022db38 main + 3660 15 dyld 0x000000018841ab98 start + 6076 ``` So basically C mode wasn't entering block 540 as expressions like `s1` (where `s1` is a struct variable) are wrapped in an `ImplicitCastExpr`, which masks the underlying `DeclRefExpr` that is actually an `lvalue`.This patch unwraps the implicit cast with E->IgnoreImpCasts() before checking isLValue(), restoring correct detection of lvalue structs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added some logs to see the difference between C++ mode and C mode and I see this
In C++ mode
in C mode
So basically C mode wasn't entering block 540 as expressions like
s1(wheres1is a struct variable) are wrapped in anImplicitCastExpr, which masks the underlyingDeclRefExprthat is actually anlvalue.This patch unwraps the implicit cast with E->IgnoreImpCasts() before checking isLValue(), restoring correct detection of lvalue structs.