[PATCH] Add [some] template argument-extraction calls to libclang & cindex

Rob Springer rspringer at google.com
Wed Oct 8 09:54:49 PDT 2014


Hi Argyrios,
Thanks a ton for the review! I think the patch is much better now, for it.

Responses:
+ The convention in the libclang API is that if the function can return an unambiguous invalid value, we do not + assert. If you document the behavior of the function ("-1 is returned") you should allow the client to depend on it for both debug and release.
+ I suggest removing the assertions in clang_Cursor_getNumTemplateArguments and clang_Cursor_getTemplateArgumentType.

Done.
One question: ...getNumTemplateArguments and other functions depend on the helper clang_Cursor_getTemplateArgument, which also has an unambiguous "bad" return ("false"), so I've removed the asserts there, as well (or else getNumTemplateArguments would still throw them). It seems like a shame, though, to lose the debugging information that was captured in the assert message ("Invalid template argument index", for example, in the prior patch revision). Is there a convention to preserve that information somehow, or is that not standard Clang practice?

+ This indicates that we are missing another function to make using the others convenient. We are missing a function to get the kind of the I'th template argument, maybe have a function return if the I'th argument is a type or value ?
+ Also could you please add some tests for this new functionality, e.g. enhance 'test/Index/index-templates.cpp' ?

That's a good call - you're absolutely right. I've added clang_Cursor_getTemplateArgumentKind and appropriate unit tests. Give them a once-over when you get a chance.

While porting this change to cindex.py, I realized that there was a lot of duplicated code for enumeration representation. I may have bitten off more than I can chew, but I at least took a stab at unifying all that somewhat. It definitely needs to be re-reviewed. :)

Again - thank you both for the reviews (updated patch coming shortly)!

Updated tests are still passing.

================
Comment at: bindings/python/tests/cindex/test_cursor.py:275
@@ +274,3 @@
+
+
+def test_get_template_argument_unsigned_value():
----------------
eliben wrote:
> One empty line here for consistency?
Done.

================
Comment at: include/clang-c/Index.h:3016
@@ +3015,3 @@
+ *
+ * If called with I = 1 or 2, 2^32 - 7 or true will be returned, respectively.
+ * For I == 0, an invalid value will be returned or an assert will be triggered.
----------------
eliben wrote:
> I think an example with an actual unsigned would be more useful here :)
That's...a good point. Done.

http://reviews.llvm.org/D5621






More information about the cfe-commits mailing list