[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-05-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 426345. rsmith added a comment. - Reimplement __builtin_dump_struct in Sema. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124221/new/ https://reviews.llvm.org/D124221 Files: clang/docs/LanguageExtensions.rst

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:420 +auto *FD = IFD ? IFD->getAnonField() : dyn_cast(D); +if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion())) + continue; rsmith wrote: > yihanaa wro

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:420 +auto *FD = IFD ? IFD->getAnonField() : dyn_cast(D); +if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion())) + continue; yihanaa wrote: > erichkeane wrote:

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D124221#3471645 , @erichkeane wrote: > In D124221#3470550 , @aaron.ballman > wrote: > >> In D124221#3469251 , @rsmith wrote: >> >>> - Take any

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-25 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment. Thank you for inviting me to a discussion on this topic. I have something very confusing as follows: - If the developer already knows all fields info in some struct, wyh doesn't he write a simple function to do the printing(like: print_struct_foo(const struct foo *); )

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D124221#3470550 , @aaron.ballman wrote: > In D124221#3469251 , @rsmith wrote: > >> In D124221#3468706 , >> @aaron.ballman wrote: >> >>> Th

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D124221#3469251 , @rsmith wrote: > In D124221#3468706 , @aaron.ballman > wrote: > >> Thank you for looking into this! I've also thought that the >> `__builtin_dump_struct` imple

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-23 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:420 +auto *FD = IFD ? IFD->getAnonField() : dyn_cast(D); +if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion())) + continue; erichkeane wrote: > rsmith wrote:

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D124221#3468706 , @aaron.ballman wrote: > Thank you for looking into this! I've also thought that the > `__builtin_dump_struct` implementation hasn't been quite as robust as it > could be. However, the primary use case for th

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for looking into this! I've also thought that the `__builtin_dump_struct` implementation hasn't been quite as robust as it could be. However, the primary use case for that builtin has been for kernel folks to debug when they have no access to an actual d

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2434 + + bool print(int arg1, int arg2, std::initializer_list fields) { + for (auto f : fields) { rsmith wrote: > erichkeane wrote: > > rsmith wrote: > > > erichkeane wrote:

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:420 +auto *FD = IFD ? IFD->getAnonField() : dyn_cast(D); +if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion())) + continue; rsmith wrote: > erichkeane wrote:

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2434 + + bool print(int arg1, int arg2, std::initializer_list fields) { + for (auto f : fields) { erichkeane wrote: > rsmith wrote: > > erichkeane wrote: > > > I'm curious as to

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. >> I think you've misunderstood; that is not required. Though with the design >> as-is, it might make sense to restrict this to being C++-only, given that >> there's not really a way to effectively use this from C. Ah, I see the example in SemaCXX. its sad we can't

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D124221#3467467 , @erichkeane wrote: > I generally just question the usefulness of this. Despite its shortcomings, > the 'dump struct' has the capability of runtime introspection into a type, > whereas this seems to require

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. I generally just question the usefulness of this. Despite its shortcomings, the 'dump struct' has the capability of runtime introspection into a type, whereas this seems to require that the user 'know' a significant amount about the type that they are introspecting

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 424379. rsmith added a comment. - Fix some code that was making incorrect assumptions that PseudoObjectExpr is only used for ObjC properties. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124221/new/ https://re

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 424350. rsmith added a comment. - Some improvements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124221/new/ https://reviews.llvm.org/D124221 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNot

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. I'm not entirely sure whether I want to pursue this -- I'd prefer to have only a single mechanism that works well in both C and C++, rather than this (which is flexible but not really usable in C) and `__builtin_dump_struct` (which is extremely inflexible but works well

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

2022-04-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added reviewers: aaron.ballman, erichkeane, yihanaa. Herald added a project: All. rsmith requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This builtin, like __builtin_dump_struct, allows for limited intro