[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

> It might be worth adding some very simple get/set tests to ensure that properties are set as intended.

clang_PrintingPolicy_setProperty is already called in c-index-test.c and covered with test/Index/print-display-names.cpp. Do you have another kind of test in mind?

I'm not sure how to best test clang_PrintingPolicy_getProperty. I don't see how to get that cleanly covered within c-index-test.c, too. After calling the setter, one could call the getter and verify that it's the same. But c-index-test shouldn't do something like this. I've added a test in LibclangTest.cpp with which I'm not too happy (an extra parse just to test a getter feels wrong).



================
Comment at: tools/libclang/CIndex.cpp:4720
+
+#define HANDLE_CASE(P, PROPERTY_NAME)                                          \
+  case CXPrintingPolicy_##PROPERTY_NAME:                                       \
----------------
jbcoe wrote:
> I’m not convinced that the code replaced by the macro is sufficiently complicated to justify use of a macro.
I find it easier to read and verify (mapping to the wrong property can't happen), but I've no strong opinion here, fine with me - reverted to non-macro case.


================
Comment at: tools/libclang/libclang.exports:365
+clang_PrintingPolicy_get
+clang_PrintingPolicy_set
+clang_PrintingPolicy_dispose
----------------
jbcoe wrote:
> clang_PrintingPolicy_setProperty and clang_PrintingPolicy_getProperty might be better names (I know the ones you have used were my earlier suggestions).
Agree!


Repository:
  rC Clang

https://reviews.llvm.org/D39903