[PATCH] clang-c: Add clang_getTypeSpelling()

Dmitri Gribenko gribozavr at gmail.com
Thu Feb 14 13:10:50 PST 2013



================
Comment at: include/clang-c/Index.h:2698-2700
@@ -2697,1 +2697,5 @@
 /**
+ * \brief Retrieve the spelling of a given CXType.
+ * \returns the spelling of the type. This string must be freed
+ * via a call to \c clang_disposeString().
+ */
----------------
(1) No need to refer to clang_disposeString -- it should be called for all CXStrings.

(2) \returns part just duplicates the brief.  It would be better to expand the \brief to explain what 'spelling' is -- a string pretty-printed according to the language syntax.

================
Comment at: tools/c-index-test/c-index-test.c:1094-1096
@@ -1085,3 +1093,5 @@
 
-static enum CXChildVisitResult PrintTypeKind(CXCursor cursor, CXCursor p,
-                                             CXClientData d) {
+static void PrintTypeWithOptions(
+  CXType T, PrintTypeOptions *PTO,
+  const char *fmt) {
+  CXString S;
----------------
There are only 3 existing tests that use `-test-print-typekind`, and these tests are very small (test/Index/vector-types.c could be actually merged into test/Index/print-typekind.c).  I think you can just rename the option to `-print-type`, rename this function to `PrintType` and just always print the type.

================
Comment at: tools/libclang/CXType.cpp:205-207
@@ +204,5 @@
+  QualType T = GetQualType(CT);
+  if (T.isNull()) {
+    return cxstring::createEmpty();
+  }
+
----------------
Please drop the braces here.


================
Comment at: tools/libclang/CXType.cpp:216
@@ +215,3 @@
+
+  return cxstring::createCXString(OS.str());
+}
----------------
Is this for trunk?  I renamed this to cxstring::creadeDup() recently.


http://llvm-reviews.chandlerc.com/D409



More information about the cfe-commits mailing list