[PATCH] D74413: [DebugInfo] Add checks for v2 directory and file name table terminators

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:165-170
   while (*OffsetPtr < EndPrologueOffset) {
     StringRef S = DebugLineData.getCStrRef(OffsetPtr);
-    if (S.empty())
+    if (S.empty()) {
+      IncludeDirsTerminated = true;
       break;
+    }
----------------
dblaikie wrote:
> Perhaps this'd be a little more terse (but not necessarily better) written as:
> 
> ```
> Terminated = S.empty();
> if (Terminated)
>   break;
> ```
> or maybe
> ```
> if ((Terminated = S.empty()))
>   break;
> ```
> 
> Not necessary if you prefer the current version.
> 
> I'd probably more strongly suggest making the variable names a bit simpler - probably just using "Terminated" for both.
Terminated +1

The variable `IncludeDirectories` below suggests what `Terminated` means.

Both the current form and 

  if ((Terminated = S.empty()))
    break;

look fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74413