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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
