[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rGee2e414d66a4: [clang][Interp] Handle sizeof() (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I think we're OK for now, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 ___ cfe

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder marked an inline comment as done. tbaeder added a comment. @erichkeane Anything else still missing here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 ___ cfe-commits mailing list cfe-commi

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/Boolean.h:50 explicit operator unsigned() const { return V; } + explicit operator int8_t() const { return V; } tbaeder wrote: > aaron.ballman wrote: > > At some point, do we want to rename t

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder added inline comments. Comment at: clang/lib/AST/Interp/Boolean.h:50 explicit operator unsigned() const { return V; } + explicit operator int8_t() const { return V; } aaron.ballman wrote: > At some point, do we want to rename this (and int) to use u

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder updated this revision to Diff 460766. tbaeder marked 11 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 Files: clang/lib/AST/Interp/Boolean.h clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/By

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + +if (!ArgType->isConstantSizeType() || +ArgType->isDependentType()) tbaeder wrote: > erichkeane wrote: > > tbaeder wrote: > > > erichkeane wrote: > > > > Documen

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + +if (!ArgType->isConstantSizeType() || +ArgType->isDependentType()) erichkeane wrote: > tbaeder wrote: > > erichkeane wrote: > > > Documentation for `isConstantS

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder updated this revision to Diff 460758. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 Files: clang/lib/AST/Interp/Boolean.h clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Opco

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + +if (!ArgType->isConstantSizeType() || +ArgType->isDependentType()) tbaeder wrote: > erichkeane wrote: > > Documentation for `isConstantSizedType` says it isn

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + +if (!ArgType->isConstantSizeType() || +ArgType->isDependentType()) erichkeane wrote: > Documentation for `isConstantSi

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

aaron.ballman added a comment. In D133934#3794389 , @tbaeder wrote: > I can see that `HandleSizeOf()` uses 1 for void and function types as a gcc > extension, but I can't reproduce that: https://godbolt.org/z/njG9zh6PM C code allows the construct: https

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + +if (!ArgType->isConstantSizeType() || +ArgType->isDependentType()) Documentation for `isConstantSizedType` says it isn't legal to call it on dependent or in

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder updated this revision to Diff 460648. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 Files: clang/lib/AST/Interp/Boolean.h clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Opco

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder added a comment. I can see that `HandleSizeOf()` uses 1 for void and function types as a gcc extension, but I can't reproduce that: https://godbolt.org/z/njG9zh6PM Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.get

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity()); + } aaron.ballman wrote: > erichkeane wrote: > > shafik wrote: > > > I

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity()); + } erichkeane wrote: > shafik wrote: > > I notice that `Handle

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane requested changes to this revision. erichkeane added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQua

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity()); + } I notice that `HandleSizeof` special cases `void` and function typ

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder updated this revision to Diff 460412. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 Files: clang/lib/AST/Interp/Boolean.h clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Opco

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:289 +QualType ArgType = E->getTypeOfArgument(); +return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType)); + } erichkeane wrote: > You probably want `getTypeSi

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:289 +QualType ArgType = E->getTypeOfArgument(); +return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType)); + } You probably want `getTypeSizeInChars`. `getT

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder added a comment. Didn't now I could just get that information from the ASTContext, nice. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder updated this revision to Diff 460409. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 Files: clang/lib/AST/Interp/Boolean.h clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Opco

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/Context.cpp:134 +llvm::Optional Context::sizeofType(QualType T) const { + if (llvm::Optional ArgT = classify(T)) +return primSize(*ArgT); This function seems pretty large for something you sh

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

tbaeder created this revision. tbaeder added reviewers: aaron.ballman, erichkeane, shafik, tahonermann. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Handle sizeof expressions. I was a bit unsure here