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 the first sizeof(void*) bytes be the size. I'd also suggest using 1 bit in CastExprBits to be a 'is base path empty', so that you don't have to allocate for it at all. ================ Comment at: include/clang/AST/Expr.h:2796 + unsigned BasePathSize : 14; + bool CastConsistency() const; ---------------- The comment there is wrong. It was wrong at 9 bits as well, since it cannot handle our implimits. I'd like to see if you can find where the implimit for direct/indirect bases is stored, and write a static-assert for it here. ================ Comment at: test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:1 +// RUN: %clang_cc1 %s -emit-llvm -o - + ---------------- First, most of this test can be further simplified. Second, I'd like to see a test that actually tests the limit without hitting the recursive template limit. ================ Comment at: test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:20 + // not fit it, so we are testing that we do fit it. + // If -ftemplate-depth= is provided, larger values (4096 and up) cause crashes + // elsewhere. ---------------- Shouldn't we cover 16k? 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
