aaron.ballman added inline comments.
================
Comment at: clang/include/clang/AST/APValue.h:512
}
- void setVector(const APValue *E, unsigned N) {
+ void ReserveVector(unsigned N) {
assert(isVector() && "Invalid accessor");
----------------
`reserveVector` per naming conventions
================
Comment at: clang/include/clang/AST/ASTContext.h:275
- /// Used to cleanups APValues stored in the AST.
- mutable llvm::SmallVector<APValue *, 0> APValueCleanups;
-
----------------
Why are you getting rid of this? It seems like we would still want these
cleaned up.
================
Comment at: clang/include/clang/AST/Expr.h:957
/// Describes the kind of result that can be trail-allocated.
+ /// Enumerator need to stay ordered by size of there storage.
enum ResultStorageKind { RSK_None, RSK_Int64, RSK_APValue };
----------------
Enumerator -> Enumerators
there -> their
================
Comment at: clang/include/clang/AST/TextNodeDumper.h:149
- const ASTContext *Context;
+ const ASTContext *Context = nullptr;
----------------
Good catch -- this pointed out a bug in the class that I've fixed in r372323,
so you'll need to rebase.
================
Comment at: clang/lib/AST/APValue.cpp:176
+ (DataSize - sizeof(MemberPointerBase)) / sizeof(CXXRecordDecl *);
+ typedef CXXRecordDecl *PathElem;
union {
----------------
Why is this no longer a pointer to `const`?
================
Comment at: clang/lib/AST/APValue.cpp:748
+APValue::LValuePathEntry *APValue::getLValuePathPtr() {
+ return ((LV *)(char *)Data.buffer)->getPath();
----------------
Can this function be marked `const`?
================
Comment at: clang/lib/AST/ASTImporter.cpp:8745
+<<<<<<< HEAD
Expected<DeclarationName> ASTImporter::HandleNameConflict(DeclarationName Name,
----------------
Conflict markers?
================
Comment at: clang/lib/AST/ASTImporter.cpp:8808
+ Result.MakeUnion();
+ auto ImpFDecl = Import(FromValue.getUnionField());
+ if (!ImpFDecl) {
----------------
Please don't use auto as the type is not spelled out in the initialization.
================
Comment at: clang/lib/AST/ASTImporter.cpp:8813
+ }
+ auto ImpValue = Import(FromValue.getUnionValue());
+ if (!ImpValue) {
----------------
Please don't use `auto` as the type is not spelled out in the initialization.
(Same elsewhere, I'll stop commenting on them.)
================
Comment at: clang/lib/AST/TextNodeDumper.cpp:691-695
+ if (Context)
+ Node->getAPValueResult().printPretty(OS, *Context, Node->getType());
+ else
+ Node->getAPValueResult().dump(OS);
+ } else {}
----------------
You'll have to rebase this too -- these changes shouldn't be needed any longer.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:11315
+ if (VDecl->isConstexpr()) {
+ Init = ConstantExpr::Create(
----------------
Can elide braces.
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9601
+ case APValue::Union: {
+ FieldDecl *FDecl =
+ cast<FieldDecl>(GetDecl(ReadDeclID(F, Record, RecordIdx)));
----------------
`auto *`
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9607-9608
+ case APValue::AddrLabelDiff: {
+ AddrLabelExpr *LHS = cast<AddrLabelExpr>(ReadExpr(F));
+ AddrLabelExpr *RHS = cast<AddrLabelExpr>(ReadExpr(F));
+ return APValue(LHS, RHS);
----------------
`auto *`
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9614
+ bool IsDerived = Record[RecordIdx++];
+ ValueDecl *Member =
+ cast<ValueDecl>(GetDecl(ReadDeclID(F, Record, RecordIdx)));
----------------
`auto *`
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9625-9631
+ uint64_t tmp = Record[RecordIdx++];
+ bool hasLValuePath = tmp & 0x1;
+ bool isLValueOnePastTheEnd = tmp & 0x2;
+ bool isExpr = tmp & 0x4;
+ bool isTypeInfo = tmp & 0x8;
+ bool isNullPtr = tmp & 0x10;
+ bool hasBase = tmp & 0x20;
----------------
These names don't match the current coding conventions.
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9634
+ QualType ElemTy;
+ assert(!isExpr || !isTypeInfo && "LValueBase cannot be both");
+ if (hasBase) {
----------------
You should add parens here to avoid diagnostics.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:5485
+ case APValue::LValue: {
+ uint64_t tmp = Value.hasLValuePath() | Value.isLValueOnePastTheEnd() << 1 |
+ Value.getLValueBase().is<const Expr *>() << 2 |
----------------
Name doesn't match coding conventions.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:5496
+ push_back(Value.getLValueBase().getVersion());
+ if (auto *E = Value.getLValueBase().dyn_cast<const Expr *>()) {
+ AddStmt(const_cast<Expr *>(E));
----------------
`const auto *`
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:5518
+ const Decl *BaseOrMember = Elem.getAsBaseOrMember().getPointer();
+ if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(BaseOrMember))
{
+ AddDeclRef(RD);
----------------
`const auto *`
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:5522
+ } else {
+ const ValueDecl *VD = cast<ValueDecl>(BaseOrMember);
+ AddDeclRef(VD);
----------------
`const auto *`
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:5538
-void ASTWriter::AddIdentifierRef(const IdentifierInfo *II, RecordDataImpl
&Record) {
+void ASTWriter::AddIdentifierRef(const IdentifierInfo *II,
+ RecordDataImpl &Record) {
----------------
Unrelated change?
================
Comment at: clang/lib/Serialization/ASTWriterStmt.cpp:42
- ASTStmtWriter(const ASTStmtWriter&) = delete;
+ ASTStmtWriter(const ASTStmtWriter &) = delete;
----------------
It seems like there's a lot of unrelated formatting changes in this file that
should not be part of the patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63640/new/
https://reviews.llvm.org/D63640
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
