[PATCH] D75646: Add an SDK attribute to DICompileUnit

[Davide Italiano via Phabricator via llvm-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Several minors. don't need another round trip before committing.



================
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.
  */
----------------
Should we say Darwin or Apple platforms?


================
Comment at: llvm/include/llvm/IR/DIBuilder.h:139-140
     /// \param SysRoot       The clang system root (value of -isysroot).
+    /// \param SDK           The SDK name. On Darwin, this is the last component
+    ///                      of the sysroot.
     DICompileUnit *
----------------
Ditto [and elsewhere if it's used, I'm not going to clutter the review repeating the same comment over and over]


================
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]),
----------------
Unrelated, but ugh about this FIXME.


================
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]));
 
----------------
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.


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

https://reviews.llvm.org/D75646