vedgy added inline comments.
================ Comment at: clang/include/clang-c/Index.h:3552 + * If the cursor does not reference a bit field declaration or if the bit + * field's width does not depend on template parameters, 0 is returned. + */ ---------------- vedgy wrote: > collinbaker wrote: > > vedgy wrote: > > > I just thought how the new API could be used in KDevelop. Currently when > > > `clang_getFieldDeclBitWidth()` is positive, e.g. 2, KDevelop shows ` : > > > 2` after the data member name in a tooltip. Ideally a > > > template-param-dependent expression (actual code) would be displayed > > > after the colon. If that's difficult to implement, `: [tparam-dependent]` > > > or `: ?` could be displayed instead. But it would be more convenient and > > > efficient to get this information by a single call to > > > `clang_getFieldDeclBitWidth()` instead of calling > > > `clang_isFieldDeclBitWidthDependent()` each time > > > `clang_getFieldDeclBitWidth()` returns `-1`. So how about returning `-2` > > > or `0` from `clang_getFieldDeclBitWidth()` instead of adding this new API? > > I understand the motivation but I don't think requiring an extra call is > > asking too much of libclang clients, and it's one extra call that doesn't > > do much work and will be called rarely so I don't see efficiency concerns. > > Without strong reasons otherwise I think it's better to be explicit here. > KDevelop calls `clang_getFieldDeclBitWidth()` for each encountered class > member declaration. `clang_isFieldDeclBitWidthDependent()` would have to be > called each time `clang_getFieldDeclBitWidth()` returns `-1`, which would be > most of the time, because few class members are bit-fields. The work this new > function does is the same as that of `clang_getFieldDeclBitWidth()` > (repeated). > > If the concern about returning `-2` from `clang_getFieldDeclBitWidth()` is > cryptic return codes, an `enum` with named constants can be introduced. > > If the concern is breaking backward compatibility for users that relied on > the returned value being positive or `-1`, then a replacement for > `clang_getFieldDeclBitWidth()` with the most convenient API should be > introduced and `clang_getFieldDeclBitWidth()` itself - deprecated. > > KDevelop simply stores the value returned by `clang_getFieldDeclBitWidth()` > in an `int16_t m_bitWidth` data member and uses it later. So if `-2` is > returned, the only place in code to adjust would be the use of this data > member. With the current `clang_isFieldDeclBitWidthDependent()` > implementation, this function would have to be called, `-2` explicitly stored > in `m_bitWidth` and the use of `m_bitWidth` would have to be adjusted just > the same. > > Have you considered potential usage of the added API in your project? Which > alternative would be more convenient to use? One more API alternative is to replace `clang_isFieldDeclBitWidthDependent()` with `clang_isBitFieldDecl()`. The usage would be more straightforward and efficient: first call `clang_isBitFieldDecl()`; if it returns true (should be rare enough), call `clang_getFieldDeclBitWidth()`; if that returns `-1`, then the bit-field width must be unknown (dependent on template parameters). Such usage would still be less convenient and efficient compared to `clang_getFieldDeclBitWidth()` returning `-2` though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130303/new/ https://reviews.llvm.org/D130303 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
