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 (number of bases, number of fields, field
TYPES). If you knew the type of your struct, THIS well, I would expect you
wouldn't NEED a builtin like this.
================
Comment at: clang/docs/LanguageExtensions.rst:2434
+
+ bool print(int arg1, int arg2, std::initializer_list<Field> fields) {
+ for (auto f : fields) {
----------------
I'm curious as to the need for the 'arg1' and 'arg2'? Also, what is the type
of the 'fields' array that works in C?
================
Comment at: clang/docs/LanguageExtensions.rst:2437
+ std::cout << f.name;
+ if (f.bitwidth)
+ std::cout << " : " << f.bitwidth;
----------------
Hmm... what is the type of f.bitwidth? A zero-length bitwidth is a valid
thing, so having it just be an 'int' doesn't seem workable.
================
Comment at: clang/docs/LanguageExtensions.rst:2460
+The ``__builtin_reflect_struct`` function provides simple reflection for a
+class, struct, or union. The first argument of the builtin should be a pointer
+to the type to reflect. The second argument ``f`` should be some callable
----------------
While I get the value of a generic reflection 'thing', this is once again
getting awful close to 'just implement reflection capabilities', at which point
this seems insufficient.
================
Comment at: clang/docs/LanguageExtensions.rst:2462
+to the type to reflect. The second argument ``f`` should be some callable
+expression, and can be a function object or an overload set. The builtin calls
+``f``, passing any further arguments ``args...`` followed by an initializer
----------------
Is the purpose of the overload set/functor so you can support multiple types
(seeing how the 'base' below works)? How does this all work in C? (or do we
just do fields in C, since they cannot have the rest?
I wonder if we would be better if we treated this function as a 'callback' or a
'callback set' that returned a discriminated union of a pre-defined type that
describes what is being returned. That way it could handle member functions,
static variables, etc.
================
Comment at: clang/docs/LanguageExtensions.rst:2464
+``f``, passing any further arguments ``args...`` followed by an initializer
+list describing the struct value.
+
----------------
Same question about init-list, how is this usable in C? It becomes a BIT more
usable in C if instead we return a single object that represents a single AST
node.
================
Comment at: clang/docs/LanguageExtensions.rst:2471
+
+The initializer list contains the following components:
+
----------------
I find myself wishing this was a more complete list.
================
Comment at: clang/docs/LanguageExtensions.rst:2473
+
+* For each direct base class, the address of the base class, in declaration
+ order.
----------------
I wonder if bases should include their virtualness or access specifier.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:420
+ auto *FD = IFD ? IFD->getAnonField() : dyn_cast<FieldDecl>(D);
+ if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion()))
+ continue;
----------------
Losing the unnamed bitfield is unfortunate, and I the 'dump struct' handles.
As is losing the anonymous struct/union.
Also, how does this handle 'record type' members? Does the user have to know
the inner type already? If they already know that much information about the
type, they wouldn't really need this, right?
================
Comment at: clang/test/CodeGenCXX/builtin-reflect-struct.cpp:24
+
+int consume(int, Expected);
+
----------------
OH! I see now. This is unfortunately quite limiting by doing it this way. At
this point the user of this builtin is required to know the layout of the
struct before calling the builtin, which seems unfortunate. Despite the
downsides of being just a 'dump function', the other builtin had the benefit of
working on an arbitrary type.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124221/new/
https://reviews.llvm.org/D124221
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
