Tyker updated this revision to Diff 283984. Tyker marked an inline comment as done. Tyker added a comment.
this a patch i had to improve dumping before your improvement to dumping. I think there is benefits to being able to dump APValues like LValues without an ASTContext even if the output will not be as good. but i agree that now that the dumper has an ASTContext during ast-dump it is hard to test. In D85144#2191440 <https://reviews.llvm.org/D85144#2191440>, @riccibruno wrote: > Thanks for finishing this. > > I don't see why you are adding `dumpPretty`; the point of the `dump` function > I added is to dump the `APValue` as the tree-like structure it is. But your > `dumpPretty` doesn't do that at all. the intent was to reuse the same printing code that is already used for diagnostics (printPretty) since it can print Member pointers and LValues. it isn't being used for printing anything that isn't a leaf(as in no child APValues) when used in AST dump. maybe the name is the issue here. > So `dump` does one thing and `dumpPretty` does another completely different > thing. i removed dumpPretty and instead added a new overload to printPretty. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85144/new/ https://reviews.llvm.org/D85144 Files: clang/include/clang/AST/APValue.h clang/include/clang/AST/PrettyPrinter.h clang/lib/AST/APValue.cpp clang/lib/AST/TextNodeDumper.cpp clang/test/AST/ast-dump-APValue-LValue.cpp clang/test/AST/ast-dump-APValue-MemPtr.cpp clang/test/AST/ast-dump-APValue-todo.cpp
Index: clang/test/AST/ast-dump-APValue-todo.cpp =================================================================== --- clang/test/AST/ast-dump-APValue-todo.cpp +++ /dev/null @@ -1,26 +0,0 @@ -// Test without serialization: -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \ -// RUN: -ast-dump %s -ast-dump-filter Test \ -// RUN: | FileCheck --strict-whitespace --match-full-lines %s -// -// Test with serialization: -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 -emit-pch -o %t %s -// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \ -// RUN: -include-pch %t -ast-dump-all -ast-dump-filter Test /dev/null \ -// RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \ -// RUN: | FileCheck --strict-whitespace --match-full-lines %s - -int i; -struct S { - int i; -}; - -void Test() { - constexpr int *pi = &i; - // CHECK: | `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} pi 'int *const' constexpr cinit - // CHECK-NEXT: | |-value: LValue <todo> - - constexpr int(S::*pmi) = &S::i; - // CHECK: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} pmi 'int (S::*const)' constexpr cinit - // CHECK-NEXT: |-value: MemberPointer <todo> -} Index: clang/test/AST/ast-dump-APValue-MemPtr.cpp =================================================================== --- /dev/null +++ clang/test/AST/ast-dump-APValue-MemPtr.cpp @@ -0,0 +1,23 @@ +// Test without serialization: +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \ +// RUN: -ast-dump %s -ast-dump-filter Test \ +// RUN: | FileCheck --strict-whitespace --match-full-lines %s +// +// TODO: Add serialization when it is finished + +int i; +struct S { + int i; +}; + +void Test() { + constexpr int(S::*pmi) = &S::i; +} +// CHECK:Dumping Test: +// CHECK-NEXT:FunctionDecl {{.*}} <{{.*}}ast-dump-APValue-MemPtr.cpp{{.*}}, line:{{.*}}> line:{{.*}} Test 'void ()' +// CHECK-NEXT:`-CompoundStmt {{.*}} <col:{{.*}}, line:{{.*}}> +// CHECK-NEXT: `-DeclStmt {{.*}} <line:{{.*}}, col:{{.*}}> +// CHECK-NEXT: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} pmi 'int (S::*const)' constexpr cinit +// CHECK-NEXT: |-value: MemberPointer &S::i +// CHECK-NEXT: `-UnaryOperator {{.*}} <col:{{.*}}, col:{{.*}}> 'int S::*' prefix '&' cannot overflow +// CHECK-NEXT: `-DeclRefExpr {{.*}} <col:{{.*}}, col:{{.*}}> 'int' lvalue Field {{.*}} 'i' 'int' Index: clang/test/AST/ast-dump-APValue-LValue.cpp =================================================================== --- /dev/null +++ clang/test/AST/ast-dump-APValue-LValue.cpp @@ -0,0 +1,21 @@ +// Test without serialization: +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \ +// RUN: -ast-dump %s -ast-dump-filter Test \ +// RUN: | FileCheck --strict-whitespace --match-full-lines %s +// +// TODO: Add serialization when it is finished + +int i; + +void Test() { + constexpr int *pi = &i; + +} +// CHECK:Dumping Test: +// CHECK-NEXT:FunctionDecl {{.*}} <{{.*}}ast-dump-APValue-LValue.cpp{{.*}}, line:{{.*}}> line:{{.*}} Test 'void ()' +// CHECK-NEXT:`-CompoundStmt {{.*}} <col:{{.*}}, line:{{.*}}> +// CHECK-NEXT: `-DeclStmt {{.*}} <line:{{.*}}, col:{{.*}}> +// CHECK-NEXT: `-VarDecl {{.*}} <col:{{.*}}, col:{{.*}}> col:{{.*}} pi 'int *const' constexpr cinit +// CHECK-NEXT: |-value: LValue &i +// CHECK-NEXT: `-UnaryOperator {{.*}} <col:{{.*}}, col:{{.*}}> 'int *' prefix '&' cannot overflow +// CHECK-NEXT: `-DeclRefExpr {{.*}} <col:{{.*}}> 'int' lvalue Var {{.*}} 'i' 'int' Index: clang/lib/AST/TextNodeDumper.cpp =================================================================== --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -493,7 +493,8 @@ return; case APValue::LValue: (void)Context; - OS << "LValue <todo>"; + OS << "LValue "; + Value.printPretty(OS, Context, Ty); return; case APValue::Array: { unsigned ArraySize = Value.getArraySize(); @@ -558,10 +559,12 @@ return; } case APValue::MemberPointer: - OS << "MemberPointer <todo>"; + OS << "MemberPointer "; + Value.printPretty(OS, Context, Ty); return; case APValue::AddrLabelDiff: - OS << "AddrLabelDiff <todo>"; + OS << "AddrLabelDiff "; + Value.printPretty(OS, Context, Ty); return; } llvm_unreachable("Unknown APValue kind!"); Index: clang/lib/AST/APValue.cpp =================================================================== --- clang/lib/AST/APValue.cpp +++ clang/lib/AST/APValue.cpp @@ -386,9 +386,10 @@ return V.convertToDouble(); } -void APValue::printPretty(raw_ostream &Out, const ASTContext &Ctx, - QualType Ty) const { - switch (getKind()) { +static void InternalPrinter(raw_ostream &Out, const APValue &Value, QualType Ty, + const ASTContext *Ctx, + const clang::PrintingPolicy &Policy) { + switch (Value.getKind()) { case APValue::None: Out << "<out of lifetime>"; return; @@ -397,33 +398,33 @@ return; case APValue::Int: if (Ty->isBooleanType()) - Out << (getInt().getBoolValue() ? "true" : "false"); + Out << (Value.getInt().getBoolValue() ? "true" : "false"); else - Out << getInt(); + Out << Value.getInt(); return; case APValue::Float: - Out << GetApproxValue(getFloat()); + Out << GetApproxValue(Value.getFloat()); return; case APValue::FixedPoint: - Out << getFixedPoint(); + Out << Value.getFixedPoint(); return; case APValue::Vector: { Out << '{'; QualType ElemTy = Ty->castAs<VectorType>()->getElementType(); - getVectorElt(0).printPretty(Out, Ctx, ElemTy); - for (unsigned i = 1; i != getVectorLength(); ++i) { + ::InternalPrinter(Out, Value.getVectorElt(0), ElemTy, Ctx, Policy); + for (unsigned i = 1; i != Value.getVectorLength(); ++i) { Out << ", "; - getVectorElt(i).printPretty(Out, Ctx, ElemTy); + ::InternalPrinter(Out, Value.getVectorElt(i), ElemTy, Ctx, Policy); } Out << '}'; return; } case APValue::ComplexInt: - Out << getComplexIntReal() << "+" << getComplexIntImag() << "i"; + Out << Value.getComplexIntReal() << "+" << Value.getComplexIntImag() << "i"; return; case APValue::ComplexFloat: - Out << GetApproxValue(getComplexFloatReal()) << "+" - << GetApproxValue(getComplexFloatImag()) << "i"; + Out << GetApproxValue(Value.getComplexFloatReal()) << "+" + << GetApproxValue(Value.getComplexFloatImag()) << "i"; return; case APValue::LValue: { bool IsReference = Ty->isReferenceType(); @@ -432,28 +433,30 @@ if (InnerTy.isNull()) InnerTy = Ty; - LValueBase Base = getLValueBase(); + APValue::LValueBase Base = Value.getLValueBase(); if (!Base) { - if (isNullPointer()) { - Out << (Ctx.getLangOpts().CPlusPlus11 ? "nullptr" : "0"); + if (Value.isNullPointer()) { + Out << (Policy.UseNullptr ? "nullptr" : "NULL"); } else if (IsReference) { - Out << "*(" << InnerTy.stream(Ctx.getPrintingPolicy()) << "*)" - << getLValueOffset().getQuantity(); + Out << "*(" << InnerTy.stream(Policy) << "*)" + << Value.getLValueOffset().getQuantity(); } else { - Out << "(" << Ty.stream(Ctx.getPrintingPolicy()) << ")" - << getLValueOffset().getQuantity(); + Out << "(" << Ty.stream(Policy) << ")" + << Value.getLValueOffset().getQuantity(); } return; } - if (!hasLValuePath()) { + if (!Value.hasLValuePath()) { // No lvalue path: just print the offset. - CharUnits O = getLValueOffset(); - CharUnits S = Ctx.getTypeSizeInChars(InnerTy); + CharUnits O = Value.getLValueOffset(); + CharUnits S; + if (Ctx) + S = Ctx->getTypeSizeInChars(InnerTy); if (!O.isZero()) { if (IsReference) Out << "*("; - if (O % S) { + if (!Ctx || O % S) { Out << "(char*)"; S = CharUnits::One(); } @@ -462,19 +465,17 @@ Out << '&'; } - if (const ValueDecl *VD = Base.dyn_cast<const ValueDecl*>()) + if (const auto *VD = Base.dyn_cast<const ValueDecl *>()) Out << *VD; else if (TypeInfoLValue TI = Base.dyn_cast<TypeInfoLValue>()) { - TI.print(Out, Ctx.getPrintingPolicy()); + TI.print(Out, Policy); } else if (DynamicAllocLValue DA = Base.dyn_cast<DynamicAllocLValue>()) { - Out << "{*new " - << Base.getDynamicAllocType().stream(Ctx.getPrintingPolicy()) << "#" + Out << "{*new " << Base.getDynamicAllocType().stream(Policy) << "#" << DA.getIndex() << "}"; } else { assert(Base.get<const Expr *>() != nullptr && "Expecting non-null Expr"); - Base.get<const Expr*>()->printPretty(Out, nullptr, - Ctx.getPrintingPolicy()); + Base.get<const Expr *>()->printPretty(Out, nullptr, Policy); } if (!O.isZero()) { @@ -488,42 +489,44 @@ // We have an lvalue path. Print it out nicely. if (!IsReference) Out << '&'; - else if (isLValueOnePastTheEnd()) + else if (Value.isLValueOnePastTheEnd()) Out << "*(&"; QualType ElemTy; - if (const ValueDecl *VD = Base.dyn_cast<const ValueDecl*>()) { + bool IsRecord = false; + if (const auto *VD = Base.dyn_cast<const ValueDecl *>()) { Out << *VD; ElemTy = VD->getType(); } else if (TypeInfoLValue TI = Base.dyn_cast<TypeInfoLValue>()) { - TI.print(Out, Ctx.getPrintingPolicy()); + TI.print(Out, Policy); ElemTy = Base.getTypeInfoType(); } else if (DynamicAllocLValue DA = Base.dyn_cast<DynamicAllocLValue>()) { - Out << "{*new " - << Base.getDynamicAllocType().stream(Ctx.getPrintingPolicy()) << "#" + Out << "{*new " << Base.getDynamicAllocType().stream(Policy) << "#" << DA.getIndex() << "}"; ElemTy = Base.getDynamicAllocType(); } else { const Expr *E = Base.get<const Expr*>(); assert(E != nullptr && "Expecting non-null Expr"); - E->printPretty(Out, nullptr, Ctx.getPrintingPolicy()); + E->printPretty(Out, nullptr, Policy); // FIXME: This is wrong if E is a MaterializeTemporaryExpr with an lvalue // adjustment. ElemTy = E->getType(); } - ArrayRef<LValuePathEntry> Path = getLValuePath(); + ArrayRef<APValue::LValuePathEntry> Path = Value.getLValuePath(); const CXXRecordDecl *CastToBase = nullptr; for (unsigned I = 0, N = Path.size(); I != N; ++I) { - if (ElemTy->getAs<RecordType>()) { + if (IsRecord || ElemTy->getAs<RecordType>()) { + IsRecord = false; // The lvalue refers to a class type, so the next path entry is a base // or member. const Decl *BaseOrMember = Path[I].getAsBaseOrMember().getPointer(); - if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(BaseOrMember)) { + if (const auto *RD = dyn_cast<CXXRecordDecl>(BaseOrMember)) { CastToBase = RD; - ElemTy = Ctx.getRecordType(RD); + ElemTy = QualType(); + IsRecord = true; } else { - const ValueDecl *VD = cast<ValueDecl>(BaseOrMember); + const auto *VD = cast<ValueDecl>(BaseOrMember); Out << "."; if (CastToBase) Out << *CastToBase << "::"; @@ -533,12 +536,12 @@ } else { // The lvalue must refer to an array. Out << '[' << Path[I].getAsArrayIndex() << ']'; - ElemTy = Ctx.getAsArrayType(ElemTy)->getElementType(); + ElemTy = cast<ArrayType>(ElemTy.getCanonicalType())->getElementType(); } } // Handle formatting of one-past-the-end lvalues. - if (isLValueOnePastTheEnd()) { + if (Value.isLValueOnePastTheEnd()) { // FIXME: If CastToBase is non-0, we should prefix the output with // "(CastToBase*)". Out << " + 1"; @@ -548,11 +551,12 @@ return; } case APValue::Array: { - const ArrayType *AT = Ctx.getAsArrayType(Ty); + const auto *AT = cast<ArrayType>(Ty.getCanonicalType()); QualType ElemTy = AT->getElementType(); Out << '{'; - if (unsigned N = getArrayInitializedElts()) { - getArrayInitializedElt(0).printPretty(Out, Ctx, ElemTy); + if (unsigned N = Value.getArrayInitializedElts()) { + ::InternalPrinter(Out, Value.getArrayInitializedElt(0), ElemTy, Ctx, + Policy); for (unsigned I = 1; I != N; ++I) { Out << ", "; if (I == 10) { @@ -560,7 +564,8 @@ Out << "..."; break; } - getArrayInitializedElt(I).printPretty(Out, Ctx, ElemTy); + ::InternalPrinter(Out, Value.getArrayInitializedElt(I), ElemTy, Ctx, + Policy); } } Out << '}'; @@ -570,14 +575,15 @@ Out << '{'; const RecordDecl *RD = Ty->castAs<RecordType>()->getDecl(); bool First = true; - if (unsigned N = getStructNumBases()) { - const CXXRecordDecl *CD = cast<CXXRecordDecl>(RD); + if (unsigned N = Value.getStructNumBases()) { + const auto *CD = cast<CXXRecordDecl>(RD); CXXRecordDecl::base_class_const_iterator BI = CD->bases_begin(); for (unsigned I = 0; I != N; ++I, ++BI) { assert(BI != CD->bases_end()); if (!First) Out << ", "; - getStructBase(I).printPretty(Out, Ctx, BI->getType()); + ::InternalPrinter(Out, Value.getStructBase(I), BI->getType(), Ctx, + Policy); First = false; } } @@ -585,8 +591,8 @@ if (!First) Out << ", "; if (FI->isUnnamedBitfield()) continue; - getStructField(FI->getFieldIndex()). - printPretty(Out, Ctx, FI->getType()); + ::InternalPrinter(Out, Value.getStructField(FI->getFieldIndex()), + FI->getType(), Ctx, Policy); First = false; } Out << '}'; @@ -594,30 +600,44 @@ } case APValue::Union: Out << '{'; - if (const FieldDecl *FD = getUnionField()) { + if (const FieldDecl *FD = Value.getUnionField()) { Out << "." << *FD << " = "; - getUnionValue().printPretty(Out, Ctx, FD->getType()); + ::InternalPrinter(Out, Value.getUnionValue(), FD->getType(), Ctx, Policy); } Out << '}'; return; case APValue::MemberPointer: // FIXME: This is not enough to unambiguously identify the member in a // multiple-inheritance scenario. - if (const ValueDecl *VD = getMemberPointerDecl()) { + if (const ValueDecl *VD = Value.getMemberPointerDecl()) { Out << '&' << *cast<CXXRecordDecl>(VD->getDeclContext()) << "::" << *VD; return; } Out << "0"; return; case APValue::AddrLabelDiff: - Out << "&&" << getAddrLabelDiffLHS()->getLabel()->getName(); + Out << "&&" << Value.getAddrLabelDiffLHS()->getLabel()->getName(); Out << " - "; - Out << "&&" << getAddrLabelDiffRHS()->getLabel()->getName(); + Out << "&&" << Value.getAddrLabelDiffRHS()->getLabel()->getName(); return; } llvm_unreachable("Unknown APValue kind!"); } +void APValue::printPretty(raw_ostream &Out, const ASTContext &Ctx, + QualType Ty) const { + ::InternalPrinter(Out, *this, Ty, &Ctx, Ctx.getPrintingPolicy()); +} + +void APValue::printPretty(raw_ostream &Out, const ASTContext *Ctx, + QualType Ty) const { + PrintingPolicy PP((LangOptions())); + if (Ctx) + PP = Ctx->getPrintingPolicy(); + + ::InternalPrinter(Out, *this, Ty, Ctx, PP); +} + std::string APValue::getAsString(const ASTContext &Ctx, QualType Ty) const { std::string Result; llvm::raw_string_ostream Out(Result); Index: clang/include/clang/AST/PrettyPrinter.h =================================================================== --- clang/include/clang/AST/PrettyPrinter.h +++ clang/include/clang/AST/PrettyPrinter.h @@ -61,9 +61,10 @@ SplitTemplateClosers(!LO.CPlusPlus11), TerseOutput(false), PolishForDeclaration(false), Half(LO.Half), MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true), - MSVCFormatting(false), ConstantsAsWritten(false), - SuppressImplicitBase(false), FullyQualifiedName(false), - PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true) {} + MSVCFormatting(false), UseNullptr(LO.CPlusPlus11), + ConstantsAsWritten(false), SuppressImplicitBase(false), + FullyQualifiedName(false), PrintCanonicalTypes(false), + PrintInjectedClassNameWithArguments(true) {} /// Adjust this printing policy for cases where it's known that we're /// printing C++ code (for instance, if AST dumping reaches a C++-only @@ -73,6 +74,7 @@ SuppressTagKeyword = true; Bool = true; UseVoidForZeroParams = false; + UseNullptr = true; } /// The number of spaces to use to indent each line. @@ -216,6 +218,9 @@ /// after template arguments. unsigned MSVCFormatting : 1; + /// Whether null pointers should be printed as nullptr or as NULL. + unsigned UseNullptr : 1; + /// Whether we should print the constant expressions as written in the /// sources. /// Index: clang/include/clang/AST/APValue.h =================================================================== --- clang/include/clang/AST/APValue.h +++ clang/include/clang/AST/APValue.h @@ -13,6 +13,7 @@ #ifndef LLVM_CLANG_AST_APVALUE_H #define LLVM_CLANG_AST_APVALUE_H +#include "clang/AST/PrettyPrinter.h" #include "clang/Basic/FixedPoint.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/APFloat.h" @@ -375,6 +376,7 @@ void dump(raw_ostream &OS, const ASTContext &Context) const; void printPretty(raw_ostream &OS, const ASTContext &Ctx, QualType Ty) const; + void printPretty(raw_ostream &OS, const ASTContext *Ctx, QualType Ty) const; std::string getAsString(const ASTContext &Ctx, QualType Ty) const; APSInt &getInt() {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits