-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[clang-repl] Lay the basic infrastructure for pretty printing of types #148701
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
@llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) ChangesThe idea is to store a type-value pair in clang::Value which is updated by the interpreter runtime. The class copies builtin types and boxes non-builtin types to provide some lifetime control. The patch enables default printers for C and C++ using a very minimalistic approach. We handle enums, arrays and user types. Once we land this we can focus on enabling user-defined pretty-printers which take control over printing of types The work started as part of https://reviews.llvm.org/D146809, then we created a giant in #84769 Patch is 36.25 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148701.diff 13 Files Affected:
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 2b9cd035623cc..f058239aabedc 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1192,6 +1192,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
bool isInSameModule(const Module *M1, const Module *M2) const;
TranslationUnitDecl *getTranslationUnitDecl() const {
+ assert(TUDecl->getMostRecentDecl() == TUDecl &&
+ "The active TU is not current one!");
return TUDecl->getMostRecentDecl();
}
void addTranslationUnitDecl() {
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 78dff1165dcf5..7ff5599a8a568 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -175,23 +175,15 @@ class Interpreter {
llvm::Expected<llvm::orc::ExecutorAddr>
getSymbolAddressFromLinkerName(llvm::StringRef LinkerName) const;
- const llvm::SmallVectorImpl<Expr *> &getValuePrintingInfo() const {
- return ValuePrintingInfo;
- }
-
- Expr *SynthesizeExpr(Expr *E);
+ std::unique_ptr<llvm::Module> GenModule(IncrementalAction *Action = nullptr);
+ PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU,
+ std::unique_ptr<llvm::Module> M = {},
+ IncrementalAction *Action = nullptr);
private:
size_t getEffectivePTUSize() const;
void markUserCodeStart();
llvm::Expected<Expr *> ExtractValueFromExpr(Expr *E);
- llvm::Expected<llvm::orc::ExecutorAddr> CompileDtorCall(CXXRecordDecl *CXXRD);
-
- CodeGenerator *getCodeGen(IncrementalAction *Action = nullptr) const;
- std::unique_ptr<llvm::Module> GenModule(IncrementalAction *Action = nullptr);
- PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU,
- std::unique_ptr<llvm::Module> M = {},
- IncrementalAction *Action = nullptr);
// A cache for the compiled destructors used to for de-allocation of managed
// clang::Values.
@@ -200,6 +192,24 @@ class Interpreter {
llvm::SmallVector<Expr *, 4> ValuePrintingInfo;
std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder;
+
+ /// @}
+ /// @name Value and pretty printing support
+ /// @{
+
+ std::string ValueDataToString(const Value &V);
+ std::string ValueTypeToString(const Value &V) const;
+
+ llvm::Expected<Expr *> convertExprToValue(Expr *E);
+
+ // When we deallocate clang::Value we need to run the destructor of the type.
+ // This function forces emission of the needed dtor.
+ llvm::Expected<llvm::orc::ExecutorAddr> CompileDtorCall(CXXRecordDecl *CXXRD);
+
+ /// @}
+ /// @name Code generation
+ /// @{
+ CodeGenerator *getCodeGen(IncrementalAction *Action = nullptr) const;
};
} // namespace clang
diff --git a/clang/include/clang/Interpreter/Value.h b/clang/include/clang/Interpreter/Value.h
index a93c0841915fc..e71e4e37e22f6 100644
--- a/clang/include/clang/Interpreter/Value.h
+++ b/clang/include/clang/Interpreter/Value.h
@@ -119,9 +119,9 @@ class REPL_EXTERNAL_VISIBILITY Value {
~Value();
void printType(llvm::raw_ostream &Out) const;
- void printData(llvm::raw_ostream &Out) const;
- void print(llvm::raw_ostream &Out) const;
- void dump() const;
+ void printData(llvm::raw_ostream &Out);
+ void print(llvm::raw_ostream &Out);
+ void dump();
void clear();
ASTContext &getASTContext();
@@ -205,6 +205,5 @@ template <> inline void *Value::as() const {
return Data.m_Ptr;
return (void *)as<uintptr_t>();
}
-
} // namespace clang
#endif
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 38cf139fa86a6..70de4a2aaa541 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -29,6 +29,7 @@ add_clang_library(clangInterpreter
InterpreterUtils.cpp
RemoteJITUtils.cpp
Value.cpp
+ InterpreterValuePrinter.cpp
${WASM_SRC}
PARTIAL_SOURCES_INTENDED
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index ed3bae59a144c..1bacf60627cee 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -264,7 +264,7 @@ class InProcessPrintingASTConsumer final : public MultiplexConsumer {
if (auto *TLSD = llvm::dyn_cast<TopLevelStmtDecl>(D))
if (TLSD && TLSD->isSemiMissing()) {
auto ExprOrErr =
- Interp.ExtractValueFromExpr(cast<Expr>(TLSD->getStmt()));
+ Interp.convertExprToValue(cast<Expr>(TLSD->getStmt()));
if (llvm::Error E = ExprOrErr.takeError()) {
llvm::logAllUnhandledErrors(std::move(E), llvm::errs(),
"Value printing failed: ");
@@ -416,6 +416,8 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> Instance,
return;
}
}
+
+ ValuePrintingInfo.resize(4);
}
Interpreter::~Interpreter() {
@@ -440,11 +442,10 @@ const char *const Runtimes = R"(
#define __CLANG_REPL__ 1
#ifdef __cplusplus
#define EXTERN_C extern "C"
- void *__clang_Interpreter_SetValueWithAlloc(void*, void*, void*);
struct __clang_Interpreter_NewTag{} __ci_newtag;
void* operator new(__SIZE_TYPE__, void* __p, __clang_Interpreter_NewTag) noexcept;
template <class T, class = T (*)() /*disable for arrays*/>
- void __clang_Interpreter_SetValueCopyArr(T* Src, void* Placement, unsigned long Size) {
+ void __clang_Interpreter_SetValueCopyArr(const T* Src, void* Placement, unsigned long Size) {
for (auto Idx = 0; Idx < Size; ++Idx)
new ((void*)(((T*)Placement) + Idx), __ci_newtag) T(Src[Idx]);
}
@@ -454,8 +455,12 @@ const char *const Runtimes = R"(
}
#else
#define EXTERN_C extern
+ EXTERN_C void *memcpy(void *restrict dst, const void *restrict src, __SIZE_TYPE__ n);
+ EXTERN_C inline void __clang_Interpreter_SetValueCopyArr(const void* Src, void* Placement, unsigned long Size) {
+ memcpy(Placement, Src, Size);
+ }
#endif // __cplusplus
-
+ EXTERN_C void *__clang_Interpreter_SetValueWithAlloc(void*, void*, void*);
EXTERN_C void __clang_Interpreter_SetValueNoAlloc(void *This, void *OutVal, void *OpaqueType, ...);
)";
@@ -470,12 +475,12 @@ Interpreter::create(std::unique_ptr<CompilerInstance> CI,
// Add runtime code and set a marker to hide it from user code. Undo will not
// go through that.
- auto PTU = Interp->Parse(Runtimes);
- if (!PTU)
- return PTU.takeError();
+ Err = Interp->ParseAndExecute(Runtimes);
+ if (Err)
+ return std::move(Err);
+
Interp->markUserCodeStart();
- Interp->ValuePrintingInfo.resize(4);
return std::move(Interp);
}
@@ -524,12 +529,11 @@ Interpreter::createWithCUDA(std::unique_ptr<CompilerInstance> CI,
return std::move(Interp);
}
+CompilerInstance *Interpreter::getCompilerInstance() { return CI.get(); }
const CompilerInstance *Interpreter::getCompilerInstance() const {
- return CI.get();
+ return const_cast<Interpreter *>(this)->getCompilerInstance();
}
-CompilerInstance *Interpreter::getCompilerInstance() { return CI.get(); }
-
llvm::Expected<llvm::orc::LLJIT &> Interpreter::getExecutionEngine() {
if (!IncrExecutor) {
if (auto Err = CreateExecutor())
@@ -610,7 +614,14 @@ Interpreter::Parse(llvm::StringRef Code) {
if (!TuOrErr)
return TuOrErr.takeError();
- return RegisterPTU(*TuOrErr);
+ PTUs.emplace_back(PartialTranslationUnit());
+ PartialTranslationUnit &LastPTU = PTUs.back();
+ LastPTU.TUPart = *TuOrErr;
+
+ if (std::unique_ptr<llvm::Module> M = GenModule())
+ LastPTU.TheModule = std::move(M);
+
+ return LastPTU;
}
static llvm::Expected<llvm::orc::JITTargetMachineBuilder>
@@ -806,13 +817,13 @@ Interpreter::GenModule(IncrementalAction *Action) {
// of the module which does not map well to CodeGen's design. To work this
// around we created an empty module to make CodeGen happy. We should make
// sure it always stays empty.
- assert(((!CachedInCodeGenModule ||
- !getCompilerInstance()->getPreprocessorOpts().Includes.empty()) ||
- (CachedInCodeGenModule->empty() &&
+ assert((!CachedInCodeGenModule ||
+ !getCompilerInstance()->getPreprocessorOpts().Includes.empty()) ||
+ ((CachedInCodeGenModule->empty() &&
CachedInCodeGenModule->global_empty() &&
CachedInCodeGenModule->alias_empty() &&
CachedInCodeGenModule->ifunc_empty())) &&
- "CodeGen wrote to a readonly module");
+ "CodeGen wrote to a readonly module");
std::unique_ptr<llvm::Module> M(CG->ReleaseModule());
CG->StartModule("incr_module_" + std::to_string(ID++), M->getContext());
return M;
@@ -828,4 +839,4 @@ CodeGenerator *Interpreter::getCodeGen(IncrementalAction *Action) const {
return nullptr;
return static_cast<CodeGenAction *>(WrappedAct)->getCodeGenerator();
}
-} // namespace clang
+} // end namespace clang
diff --git a/clang/lib/Interpreter/InterpreterUtils.cpp b/clang/lib/Interpreter/InterpreterUtils.cpp
index 45f6322b8461e..a19f96c80b94f 100644
--- a/clang/lib/Interpreter/InterpreterUtils.cpp
+++ b/clang/lib/Interpreter/InterpreterUtils.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "InterpreterUtils.h"
+#include "clang/AST/QualTypeNames.h"
namespace clang {
@@ -81,7 +82,7 @@ NamedDecl *LookupNamed(Sema &S, llvm::StringRef Name,
else {
const DeclContext *PrimaryWithin = nullptr;
if (const auto *TD = dyn_cast<TagDecl>(Within))
- PrimaryWithin = llvm::dyn_cast_or_null<DeclContext>(TD->getDefinition());
+ PrimaryWithin = dyn_cast_if_present<DeclContext>(TD->getDefinition());
else
PrimaryWithin = Within->getPrimaryContext();
@@ -97,15 +98,16 @@ NamedDecl *LookupNamed(Sema &S, llvm::StringRef Name,
R.resolveKind();
if (R.isSingleResult())
- return llvm::dyn_cast<NamedDecl>(R.getFoundDecl());
+ return dyn_cast<NamedDecl>(R.getFoundDecl());
return nullptr;
}
std::string GetFullTypeName(ASTContext &Ctx, QualType QT) {
+ QualType FQT = TypeName::getFullyQualifiedType(QT, Ctx);
PrintingPolicy Policy(Ctx.getPrintingPolicy());
Policy.SuppressScope = false;
Policy.AnonymousTagLocations = false;
- return QT.getAsString(Policy);
+ return FQT.getAsString(Policy);
}
} // namespace clang
diff --git a/clang/lib/Interpreter/InterpreterUtils.h b/clang/lib/Interpreter/InterpreterUtils.h
index c7b405b486d93..fbf9814b0d4a7 100644
--- a/clang/lib/Interpreter/InterpreterUtils.h
+++ b/clang/lib/Interpreter/InterpreterUtils.h
@@ -45,7 +45,7 @@ NamespaceDecl *LookupNamespace(Sema &S, llvm::StringRef Name,
const DeclContext *Within = nullptr);
NamedDecl *LookupNamed(Sema &S, llvm::StringRef Name,
- const DeclContext *Within);
+ const DeclContext *Within = nullptr);
std::string GetFullTypeName(ASTContext &Ctx, QualType QT);
} // namespace clang
diff --git a/clang/lib/Interpreter/InterpreterValuePrinter.cpp b/clang/lib/Interpreter/InterpreterValuePrinter.cpp
index 3e7e32b2e8557..ab5edac9029dc 100644
--- a/clang/lib/Interpreter/InterpreterValuePrinter.cpp
+++ b/clang/lib/Interpreter/InterpreterValuePrinter.cpp
@@ -18,6 +18,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Interpreter/Interpreter.h"
#include "clang/Interpreter/Value.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Sema.h"
@@ -25,11 +26,333 @@
#include "llvm/Support/raw_ostream.h"
#include <cassert>
-
#include <cstdarg>
+#include <sstream>
+#include <string>
+
+#define DEBUG_TYPE "interp-value"
+
+using namespace clang;
+
+static std::string DeclTypeToString(const QualType &QT, NamedDecl *D) {
+ std::string Str;
+ llvm::raw_string_ostream SS(Str);
+ if (QT.hasQualifiers())
+ SS << QT.getQualifiers().getAsString() << " ";
+ SS << D->getQualifiedNameAsString();
+ return Str;
+}
+
+static std::string QualTypeToString(ASTContext &Ctx, QualType QT) {
+ PrintingPolicy Policy(Ctx.getPrintingPolicy());
+ // Print the Allocator in STL containers, for instance.
+ Policy.SuppressDefaultTemplateArgs = false;
+ Policy.SuppressUnwrittenScope = true;
+ // Print 'a<b<c> >' rather than 'a<b<c>>'.
+ Policy.SplitTemplateClosers = true;
+
+ struct LocalPrintingPolicyRAII {
+ ASTContext &Context;
+ PrintingPolicy Policy;
+
+ LocalPrintingPolicyRAII(ASTContext &Ctx, PrintingPolicy &PP)
+ : Context(Ctx), Policy(Ctx.getPrintingPolicy()) {
+ Context.setPrintingPolicy(PP);
+ }
+ ~LocalPrintingPolicyRAII() { Context.setPrintingPolicy(Policy); }
+ } X(Ctx, Policy);
+
+ const QualType NonRefTy = QT.getNonReferenceType();
+
+ if (const auto *TTy = llvm::dyn_cast<TagType>(NonRefTy))
+ return DeclTypeToString(NonRefTy, TTy->getDecl());
+
+ if (const auto *TRy = dyn_cast<RecordType>(NonRefTy))
+ return DeclTypeToString(NonRefTy, TRy->getDecl());
+
+ const QualType Canon = NonRefTy.getCanonicalType();
+
+ // FIXME: How a builtin type can be a function pointer type?
+ if (Canon->isBuiltinType() && !NonRefTy->isFunctionPointerType() &&
+ !NonRefTy->isMemberPointerType())
+ return Canon.getAsString(Ctx.getPrintingPolicy());
+
+ if (const auto *TDTy = dyn_cast<TypedefType>(NonRefTy)) {
+ // FIXME: TemplateSpecializationType & SubstTemplateTypeParmType checks
+ // are predominately to get STL containers to print nicer and might be
+ // better handled in GetFullyQualifiedName.
+ //
+ // std::vector<Type>::iterator is a TemplateSpecializationType
+ // std::vector<Type>::value_type is a SubstTemplateTypeParmType
+ //
+ QualType SSDesugar = TDTy->getLocallyUnqualifiedSingleStepDesugaredType();
+ if (llvm::isa<SubstTemplateTypeParmType>(SSDesugar))
+ return GetFullTypeName(Ctx, Canon);
+ else if (llvm::isa<TemplateSpecializationType>(SSDesugar))
+ return GetFullTypeName(Ctx, NonRefTy);
+ return DeclTypeToString(NonRefTy, TDTy->getDecl());
+ }
+ return GetFullTypeName(Ctx, NonRefTy);
+}
+
+static std::string EnumToString(const Value &V) {
+ std::string Str;
+ llvm::raw_string_ostream SS(Str);
+ ASTContext &Ctx = const_cast<ASTContext &>(V.getASTContext());
+
+ QualType DesugaredTy = V.getType().getDesugaredType(Ctx);
+ const EnumType *EnumTy = DesugaredTy.getNonReferenceType()->getAs<EnumType>();
+ assert(EnumTy && "Fail to cast to enum type");
+
+ EnumDecl *ED = EnumTy->getDecl();
+ uint64_t Data = V.getULongLong();
+ bool IsFirst = true;
+ llvm::APSInt AP = Ctx.MakeIntValue(Data, DesugaredTy);
+
+ for (auto I = ED->enumerator_begin(), E = ED->enumerator_end(); I != E; ++I) {
+ if (I->getInitVal() == AP) {
+ if (!IsFirst)
+ SS << " ? ";
+ SS << "(" + I->getQualifiedNameAsString() << ")";
+ IsFirst = false;
+ }
+ }
+ llvm::SmallString<64> APStr;
+ AP.toString(APStr, /*Radix=*/10);
+ SS << " : " << QualTypeToString(Ctx, ED->getIntegerType()) << " " << APStr;
+ return Str;
+}
+
+static std::string FunctionToString(const Value &V, const void *Ptr) {
+ std::string Str;
+ llvm::raw_string_ostream SS(Str);
+ SS << "Function @" << Ptr;
+
+ const DeclContext *PTU = V.getASTContext().getTranslationUnitDecl();
+ // Find the last top-level-stmt-decl. This is a forward iterator but the
+ // partial translation unit should not be large.
+ const TopLevelStmtDecl *TLSD = nullptr;
+ for (const Decl *D : PTU->noload_decls())
+ if (isa<TopLevelStmtDecl>(D))
+ TLSD = cast<TopLevelStmtDecl>(D);
+
+ // Get __clang_Interpreter_SetValueNoAlloc(void *This, void *OutVal, void
+ // *OpaqueType, void *Val);
+ const FunctionDecl *FD = nullptr;
+ if (auto *InterfaceCall = llvm::dyn_cast<CallExpr>(TLSD->getStmt())) {
+ const auto *Arg = InterfaceCall->getArg(/*Val*/ 3);
+ // Get rid of cast nodes.
+ while (const CastExpr *CastE = llvm::dyn_cast<CastExpr>(Arg))
+ Arg = CastE->getSubExpr();
+ if (const DeclRefExpr *DeclRefExp = llvm::dyn_cast<DeclRefExpr>(Arg))
+ FD = llvm::dyn_cast<FunctionDecl>(DeclRefExp->getDecl());
+
+ if (FD) {
+ SS << '\n';
+ const clang::FunctionDecl *FDef;
+ if (FD->hasBody(FDef))
+ FDef->print(SS);
+ }
+ }
+ return Str;
+}
+
+static std::string AddressToString(const void *Ptr, char Prefix) {
+ std::string Str;
+ llvm::raw_string_ostream SS(Str);
+ if (!Ptr)
+ return Str;
+ SS << Prefix << Ptr;
+ return Str;
+}
+
+static std::string CharPtrToString(const char *Ptr) {
+ if (!Ptr)
+ return "0";
+
+ std::string result = "\"";
+ result += Ptr;
+ result += '"';
+ return result;
+}
namespace clang {
+struct ValueRef : public Value {
+ ValueRef(Interpreter *In, void *Ty) : Value(In, Ty) {
+ // Tell the base class to not try to deallocate if it manages the value.
+ IsManuallyAlloc = false;
+ }
+ void setData(long double D) { Data.m_LongDouble = D; }
+};
+
+std::string Interpreter::ValueDataToString(const Value &V) {
+ Sema &S = getCompilerInstance()->getSema();
+ ASTContext &Ctx = S.getASTContext();
+
+ QualType QT = V.getType();
+
+ if (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(QT)) {
+ QualType ElemTy = CAT->getElementType();
+ size_t ElemCount = Ctx.getConstantArrayElementCount(CAT);
+ const Type *BaseTy = CAT->getBaseElementTypeUnsafe();
+ size_t ElemSize = Ctx.getTypeSizeInChars(BaseTy).getQuantity();
+
+ // Treat null terminated char arrays as strings basically.
+ if (ElemTy->isCharType()) {
+ char last = *(char *)(((uintptr_t)V.getPtr()) + ElemCount * ElemSize - 1);
+ if (last == '\0')
+ return CharPtrToString((char *)V.getPtr());
+ }
+
+ std::string result = "{ ";
+ for (unsigned Idx = 0, N = CAT->getZExtSize(); Idx < N; ++Idx) {
+ ValueRef InnerV = ValueRef(this, ElemTy.getAsOpaquePtr());
+ if (ElemTy->isBuiltinType()) {
+ // Single dim arrays, advancing.
+ uintptr_t offset = (uintptr_t)V.getPtr() + Idx * ElemSize;
+ InnerV.setData(*(long double *)offset);
+ } else {
+ // Multi dim arrays, position to the next dimension.
+ size_t Stride = ElemCount / N;
+ uintptr_t offset = ((uintptr_t)V.getPtr()) + Idx * Stride * ElemSize;
+ InnerV.setPtr((void *)offset);
+ }
+
+ result += ValueDataToString(InnerV);
+
+ // Skip the \0 if the char types
+ if (Idx < N - 1)
+ result += ", ";
+ }
+ result += " }";
+ return result;
+ }
+
+ QualType DesugaredTy = QT.getDesugaredType(Ctx);
+ QualType NonRefTy = DesugaredTy.getNonReferenceType();
+
+ // FIXME: Add support for user defined printers.
+ // LookupResult R = LookupUserDefined(S, QT);
+ // if (!R.empty())
+ // return CallUserSpecifiedPrinter(R, V);
+
+ // If it is a builtin type dispatch to the builtin overloads.
+ if (auto *BT = DesugaredTy.getCanonicalType()->getAs<BuiltinType>()) {
+
+ auto formatFloating = [](auto val, char suffix = '\0') -> std::string {
+ std::string out;
+ llvm::raw_string_ostream ss(out);
+
+ if (std::isnan(val) || std::isinf(val)) {
+ ss << llvm::format("%g", val);
+ return ss.str();
+ }
+ if (val == static_cast<decltype(val)>(static_cast<int64_t>(val)))
+ ss << llvm::format("%.1f", val);
+ else if (std::abs(val) < 1e-4 || std::abs(val) > 1e6 || suffix == 'f')
+ ss << llvm::format("%#.6g", val);
+ else if (suffix == 'L')
+ ss << llvm::format("%#.12Lg", val);
+ else
+ ss << llvm::format("%#.8g", val);
+
+ if (suffix != '\0')
+ ss << suffix;
+ return ss.str();
+ };
+
+ std::string str;
+ llvm::raw_string_ostream ss(str);
+ switch (BT->getKind()) {
+ default:
+ return "{ error: unknown builtin type '" + std::to_string(BT->getKind()) +
+ " '}";
+ case clang::BuiltinType::Bool:
+ ss << ((V.getBool()) ? "true" : "false");
+ return str;
+ case clang::BuiltinType::Char_S:
+ ss << '\'' << V.getChar_S() << '\'';
+ return str;
+ case clang::BuiltinType::SChar:
+ ss << '\'' << V.getSChar() << '\'';
+ return str;
+ case clang::BuiltinType::Char_U:
+ ss << '\'' << V.getChar_U() << '\'';
+ return str;
+ case clang::Buil...
[truncated]
|
return CharPtrToString((char *)V.getPtr()); | ||
} | ||
|
||
std::string result = "{ "; |
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.
You should take a pass over things and make sure identifiers meet the usual naming conventions.
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.
I think I addressed this.
The idea is to store a type-value pair in clang::Value which is updated by the interpreter runtime. The class copies builtin types and boxes non-builtin types to provide some lifetime control. The patch enables default printers for C and C++ using a very minimalistic approach. We handle enums, arrays and user types. Once we land this we can focus on enabling user-defined pretty-printers which take control over printing of types The work started as part of https://reviews.llvm.org/D146809, then we created a giant in llvm#84769
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
Precommit CI found some crashing issues that will need to be addressed, and I had some more testing requests to make sure we handle basics properly.
float f = 4.2f; f | ||
// CHECK-NEXT: (float) 4.20000f | ||
|
||
double d = 4.21; d | ||
// CHECK-NEXT: (double) 4.2100000 | ||
|
||
long double tau = 6.2831853; tau | ||
// CHECK-NEXT: (long double) 6.28318530000L |
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.
Will these values be sensitive to the target architecture the test is run on? (I'm thinking about things like PPC double double or x87 long double.)
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.
I expect this to print long double
however I might be proven wrong by the llvm buildbot jungle. We'll see :)
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.
I also expect it to print (long double)
but what I was talking about was that I don't think 6.28318530000
is a value that all targets will print.
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.
I am not sure how to address this comment at that point. Let's see if we will have some failures and figure out how to adjust to them.
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.
Yeah, I expect this will fail in post-commit CI testing, so keep an eye on that when the changes land. But I don't think that blocks landing the changes.
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.
Makes sense. I am aiming to land this over the weekend to have time to meditate on these things :)
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!
float f = 4.2f; f | ||
// CHECK-NEXT: (float) 4.20000f | ||
|
||
double d = 4.21; d | ||
// CHECK-NEXT: (double) 4.2100000 | ||
|
||
long double tau = 6.2831853; tau | ||
// CHECK-NEXT: (long double) 6.28318530000L |
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.
Yeah, I expect this will fail in post-commit CI testing, so keep an eye on that when the changes land. But I don't think that blocks landing the changes.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/38669 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/13089 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/164/builds/11754 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/9047 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/42/builds/5414 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/136/builds/4637 Here is the relevant piece of the build log for the reference
|
Working on understanding the issues... |
This change is a follow-up of #148701 where clang-s390x-linux and clang-s390x-linux-lnt failed.
This change is a follow-up of llvm/llvm-project#148701 where clang-s390x-linux and clang-s390x-linux-lnt failed.
I added a fix for the system-z, however, I believe I still a maintainer to confirm this is not a big-endian related issue. I realize I do not understand the sanitizer failures. I ran the tests with valgrind and it seems happy with it. @vitalybuka what should I do here? |
While waiting for the bot owners it seems that this is not a major issue due to the big endianness of the systemz platform. Instead it looks like we are not modelling something well for enum types. Probably `va_arg` has a bug for that platform or similar. The asan failure seems to be a crash in asan and maybe related to the issues we've mentioned in #102858. This patch should appease the bots that were broken by #148701
0a463bd should bring piece in the galaxy. However, I am puzzled by the system-z (s390) -- it is a big endian -- it is a good test bed for the conformance for low-level primitives such as For enums of an llvm-project/clang/lib/Interpreter/InterpreterValuePrinter.cpp Lines 696 to 698 in 0a463bd
Perhaps there is some bug in handling variadic arguments on that platform. I do not have access to a big-endian platform to verify my guess. Last time I tried alpine + qemu to emulate a big-endian platform it was unusable for a large project such as llvm. I still need a clarification on the |
While waiting for the bot owners it seems that this is not a major issue due to the big endianness of the systemz platform. Instead it looks like we are not modelling something well for enum types. Probably `va_arg` has a bug for that platform or similar. The asan failure seems to be a crash in asan and maybe related to the issues we've mentioned in llvm/llvm-project#102858. This patch should appease the bots that were broken by llvm/llvm-project#148701
The idea is to store a type-value pair in clang::Value which is updated by the interpreter runtime. The class copies builtin types and boxes non-builtin types to provide some lifetime control.
The patch enables default printers for C and C++ using a very minimalistic approach. We handle enums, arrays and user types. Once we land this we can focus on enabling user-defined pretty-printers which take control over printing of types
The work started as part of https://reviews.llvm.org/D146809, then we created a giant in #84769