[PATCH] D75646: Add an SDK attribute to DICompileUnit

[Adrian Prantl via Phabricator via llvm-commits]

aprantl marked 3 inline comments as done.
aprantl added inline comments.


================
Comment at: llvm/include/llvm-c/DebugInfo.h:255-256
  * \param SysRootLen      The length of the C string passed to \c SysRoot.
+ * \param SDK           The SDK. On Darwin, the last component of the sysroot.
+ * \param SDKLen        The length of the C string passed to \c SDK.
  */
----------------
davide wrote:
> Should we say Darwin or Apple platforms?
I think Darwin is more precise here.


================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1475-1476
         Record.size() <= 18 ? 0 : Record[18],
         false, // FIXME: https://reviews.llvm.org/rGc51b45e32ef7f35c11891f60871aa9c2c04cd991
                // Record.size() <= 19 ? 0 : Record[19],
+        Record.size() <= 20 ? nullptr : getMDString(Record[20]),
----------------
davide wrote:
> Unrelated, but ugh about this FIXME.
Let's ping @dblaikie about that :-)


================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1477-1478
                // Record.size() <= 19 ? 0 : Record[19],
-        Record.size() <= 20 ? nullptr : getMDString(Record[20]));
+        Record.size() <= 20 ? nullptr : getMDString(Record[20]),
+        Record.size() <= 21 ? nullptr : getMDString(Record[21]));
 
----------------
davide wrote:
> Not your fault but it would be great if. we could put a comment next to each line so we don't have to map back numbers to attributes.
That's becoming a good idea!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75646/new/

https://reviews.llvm.org/D75646