[PATCH] D32389: [libclang] Expose some target information via the C API.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

It looks good, I have a couple of comments:



================
Comment at: clang/include/clang-c/Index.h:1573
+CINDEX_LINKAGE void
+clang_TargetInfo_dispose(CXTargetInfo info);
+
----------------
Please capitalize `Info` here and in the other parameter lists.


================
Comment at: clang/include/clang-c/Index.h:1584
+/**
+ * \brief Get the pointer width of the target.
+ *
----------------
You could mention that the function returns the width in bits.


================
Comment at: clang/test/Index/target-info.c:4
+// CHECK: PointerWidth: 32
+// RUN: c-index-test -test-print-target-info %s --target=x86_64-unknown-linux-gnu | FileCheck --check-prefix=CHECK-1 %s
+// CHECK-1: TargetTriple: x86_64-unknown-linux-gnu
----------------
NIT: Please put both RUN lines at the top of the file.


================
Comment at: clang/tools/libclang/CIndex.cpp:4035
+  CXTranslationUnit CTUnit = TargetInfo->TranslationUnit;
+  assert(!isNotUsableTU(CTUnit));
+
----------------
Please add a message to the `assert`  (e.g. `&& "message"`).


================
Comment at: clang/tools/libclang/CIndex.cpp:4048
+  CXTranslationUnit CTUnit = TargetInfo->TranslationUnit;
+  assert(!isNotUsableTU(CTUnit));
+
----------------
Please add an assertion message here as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D32389