[PATCH] D38504: [llvm-dsymutil] Add support for __swift_ast MachO DWARF section

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: test/tools/dsymutil/swift-ast.test:1
+RUN: rm -rf %t
+RUN: mkdir -p %t
----------------
Why is this necessary? 


================
Comment at: test/tools/dsymutil/swift-ast.test:2
+RUN: rm -rf %t
+RUN: mkdir -p %t
+RUN: llvm-dsymutil -oso-prepend-path %p %p/Inputs/swift_ast -o %t/swift_ast.dSYM -verbose | FileCheck %s --check-prefix=DSYMUTIL
----------------
You can use `%T` for a temporary directory unique to your test. 


================
Comment at: test/tools/dsymutil/swift-ast.test:9
+
+compiled for x86_64.
+
----------------
For consistency with the other tests, please include the actual compile command. (i.e. `Compile with:`)


================
Comment at: tools/dsymutil/DwarfLinker.cpp:716
+void DwarfStreamer::emitSwiftAST(const std::vector<MemoryBufferRef> &Buffers) {
+  MS->SwitchSection(MOFI->getDwarfSwiftASTSection());
+  for (auto Buf : Buffers) {
----------------
The swift AST section must be 32-bit aligned. Our internal code has `Section->setAlignment(1 << 5);`. though I think the operand is in bytes (rather than bits). Maybe I'm wrong or maybe this is because LLDB has stricter requirements. Either way we'd want to do the same here.


================
Comment at: tools/dsymutil/DwarfLinker.cpp:717
+  MS->SwitchSection(MOFI->getDwarfSwiftASTSection());
+  for (auto Buf : Buffers) {
+    MS->EmitBytes(Buf.getBuffer());
----------------
No {} around single line if/for.


https://reviews.llvm.org/D38504