aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaChecking.cpp:732-733 + int RDKind = RD->isClass() ? 0 : (RD->isStruct() ? 1 : 2); + S.Diag(PtrArg->getBeginLoc(), diag::err_builtin_dump_struct_too_complex) + << RDKind << RD->getName(); + return ExprError(); ---------------- rsmith wrote: > aaron.ballman wrote: > > This will correctly handle diagnosing a gigantic anonymous struct. > Producing an error here seems likely to eventually cause problems in practice > for some users: people are using `__builtin_dump_struct` in generic code for > reflection purposes, not just for debugging, and this will cause us to start > rejecting complex generic code. > > Instead of rejecting, can we produce a tree of `PseudoObjectExpr`s if we have > too many steps to store in a single expression? > Producing an error here seems likely to eventually cause problems in practice > for some users: people are using __builtin_dump_struct in generic code for > reflection purposes, not just for debugging, and this will cause us to start > rejecting complex generic code. > > Instead of rejecting, can we produce a tree of PseudoObjectExprs if we have > too many steps to store in a single expression? I think that requires wider discussion -- I don't think `__builtin_dump_struct` is a reasonable interface we want to support for reflection (in fact, I'd argue it's an explicit non-goal, the same as reflection via `-ast-dump`). Compile-time reflection is something we're likely to need to support more intentionally and I don't think we're going to want to use this as an interface for it or have to maintain it as a reflection tool long-term. As such, I think producing a tree of `PseudoObjectExpr`s is overkill; you can quote me on this a few years from now when we're laughing at its quaintness, but "16k fields of debug output is enough for anyone" for a debugging interface. (That said, I think we should be considering what support we want to add to the compiler for reflection in service of the work being done in WG21 on the topic -- if `__builtin_dump_struct` is being used for reflection in practice, it would be nice to give people a supported, more ergonomic interface for it that we can use for a future version of C++.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158296/new/ https://reviews.llvm.org/D158296 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
