[PATCH] D42765: [DebugInfo] Support DWARFv5 source code embedding extension

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

It seems a little counter-intuitive for embedded source to have a filename, but I guess that's so the debugger has a familiar-looking handle to refer to it?



================
Comment at: include/llvm/IR/DIBuilder.h:22
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SetVector.h"
----------------
If you have Optional you can remove None.h.


================
Comment at: include/llvm/MC/MCStreamer.h:20
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
----------------
Optional requires None so you shouldn't need to include None.h explicitly.


================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1353
   case bitc::METADATA_FILE: {
-    if (Record.size() != 3 && Record.size() != 5)
+    if (Record.size() < 3 || Record.size() > 6)
       return error("Invalid record");
----------------
This used to error out if size == 4, now it does not?


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:12
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallString.h"
----------------
Optional requires None so you shouldn't need to include None.h explicitly.


================
Comment at: lib/IR/DebugInfo.cpp:19
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
----------------
This is the only change to DebugInfo.cpp?  You shouldn't have to do that.


================
Comment at: lib/MC/MCAsmStreamer.cpp:10
 
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
----------------
Optional requires None so I think you don't need to include None.h explicitly.


================
Comment at: lib/MC/MCDwarf.cpp:14
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
----------------
Adding Optional means you can remove None.h.


================
Comment at: test/DebugInfo/X86/generate-odr-hash.ll:128
+; CHECK: file_names{{.*}}
+; CHECK: name: "bar.cpp"
 ; CHECK-NOT: file_names[
----------------
you don't need the trailing `{{.*}}` on these lines.  Also, use `SINGLE-NEXT` and `CHECK-NEXT`  to check the filenames?


================
Comment at: test/DebugInfo/X86/generate-odr-hash.ll:141
+; FISSION: file_names{{.*}}
+; FISSION: name: "bar.cpp"
 ; FISSION-NOT: file_names[
----------------
Don't need the trailing `{{.*}}` on these checks.  Also, use `FISSION-NEXT` to check the filenames?



================
Comment at: test/Linker/subprogram-linkonce-weak.ll:162
+; DWWL-LABEL: file_names[ 1]{{.*}}
+; DWWL: name: "bar.c"
 ; DWWL:        2 0 1 0 0 is_stmt prologue_end
----------------
Don't need the trailing `{{.*}}` on all the above.  Also use `-NEXT` to check the filenames?


https://reviews.llvm.org/D42765