[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

vsapsai added a comment. Disabling test in https://reviews.llvm.org/D54132. Repository: rC Clang https://reviews.llvm.org/D50050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

george.karpenkov added a subscriber: vsapsai. george.karpenkov added a comment. @lebedev.ri yeah ASAN is making stack frame size larger. It seems @vsapsai is working on disabling this test under ASAN. Repository: rC Clang https://reviews.llvm.org/D50050 ___

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri added a comment. In https://reviews.llvm.org/D50050#1287780, @george.karpenkov wrote: > @lebedev.ri @erichkeane The test fails for me on macOS whenever asan and > ubsan are both enabled. > The failure is stack overflow at stack frame 943 > (? maybe asan usage enforces lower stack

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

george.karpenkov added a comment. @lebedev.ri @erichkeane The test fails for me on macOS whenever asan and ubsan are both enabled. The failure is stack overflow at stack frame 943 (? maybe asan usage enforces lower stack size?) Repository: rC Clang https://reviews.llvm.org/D50050 ___

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

This revision was automatically updated to reflect the committed changes. Closed by commit rC338489: [AST] CastExpr: BasePathSize is not large enough. (authored by lebedevri, committed by ). Repository: rC Clang https://reviews.llvm.org/D50050 Files: include/clang/AST/Expr.h include/clang

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri added a comment. In https://reviews.llvm.org/D50050#1183341, @rjmccall wrote: > LGTM. Thank you for the review. I'll land in ~+9 hours or so, since i did not hear from @erichkeane back yet.. Repository: rC Clang https://reviews.llvm.org/D50050

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri updated this revision to Diff 158379. lebedev.ri added a comment. Drop pointless cast of zero. Repository: rC Clang https://reviews.llvm.org/D50050 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/ExprObjC.h include/clang/AST/Stmt.h lib/AST/E

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Comment at: include/clang/AST/Expr.h:2791 +public: + using BasePathSizeTy = unsigned short; + static_assert(std::numeric_limits::max() >= 16384,

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri updated this revision to Diff 158344. lebedev.ri marked 6 inline comments as done. lebedev.ri added a comment. Address some of @erichkeane review notes. Repository: rC Clang https://reviews.llvm.org/D50050 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/c

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri added inline comments. Comment at: include/clang/AST/Expr.h:2791 +public: + using BasePathSizeTy = unsigned short; + static_assert(std::numeric_limits::max() >= 16384, erichkeane wrote: > I'll let @rjmccall comment here, but I think I'd be willing to

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

erichkeane added inline comments. Comment at: include/clang/AST/Expr.h:2805 + } + BasePathSizeTy &BasePathSize(); + erichkeane wrote: > Why is this a reference? This seems odd... It seems that the const-cast > magic above shouldn't be necessary either. Lookin

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

erichkeane added inline comments. Comment at: include/clang/AST/Expr.h:2791 +public: + using BasePathSizeTy = unsigned short; + static_assert(std::numeric_limits::max() >= 16384, I'll let @rjmccall comment here, but I think I'd be willing to give up 2 more byt

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri added inline comments. Comment at: test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:1 +// RUN: %clang_cc1 %s -emit-llvm -o - + erichkeane wrote: > First, most of this test can be further simplified. Second, I'd like to see > a test that actually t

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri updated this revision to Diff 158311. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. - Store/prepend it into `llvm::TrailingObjects<>` - Use `unsigned short` - Assert that the storage is at least large enough to be compliant with implimits

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

erichkeane added a comment. Its not that it needs to be 'at least 9 bits', IMO 9 bits was too small as well. What I'd prefer to this solution is similar to what John McCall said on PR38356: Add the size to the trailing allocated space. Check out the 'create' methods for all of these, and make

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, rsmith, erichkeane. https://reviews.llvm.org/rC337815 / https://reviews.llvm.org/D49508 had to cannibalize one bit of `CastExprBitfields::BasePathSize` in order to squeeze `PartOfExplicitCast` boolean. That reduced the maxim