Skip to content

[NFC] Address review feedback from PR #77004 #77134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

rastogishubham
Copy link
Contributor

Accidentally didn't commit the review feedback before merging the PR

Accidentally didn't commit the review feedback before merging the PR
@rastogishubham rastogishubham merged commit f22dc88 into llvm:main Jan 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-debuginfo

Author: Shubham Sandeep Rastogi (rastogishubham)

Changes

Accidentally didn't commit the review feedback before merging the PR


Full diff: https://github.com/llvm/llvm-project/pull/77134.diff

2 Files Affected:

  • (modified) llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp (+2-2)
  • (modified) llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml (+2-3)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 18377815c78b3e..d25b732fdba3f0 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -944,8 +944,8 @@ void DWARFVerifier::verifyDebugLineRows() {
       // row, do not verify the file index, this is a line table of an empty
       // file with an end_sequence, but the DWARF standard sets the file number
       // to 1 by default, otherwise verify file index.
-      if (!(LineTable->Prologue.FileNames.size() == 0 &&
-            LineTable->Rows.size() == 1) &&
+      if ((LineTable->Prologue.FileNames.size() ||
+           LineTable->Rows.size() != 1) &&
           !LineTable->hasFileAtIndex(Row.File)) {
         ++NumDebugLineErrors;
         error() << ".debug_line["
diff --git a/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml b/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml
index e43bb880821623..fc25fceef764f9 100644
--- a/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/verify-no-file.yaml
@@ -1,6 +1,5 @@
-# RUN: rm -rf %t && mkdir -p %t
-# RUN: yaml2obj %s -o %t/test.o
-# RUN: llvm-dwarfdump --debug-line --verify %t/test.o 2>&1 | FileCheck %s
+# RUN: yaml2obj %s -o %t.o
+# RUN: llvm-dwarfdump --debug-line --verify %t.o 2>&1 | FileCheck %s
 
 # CHECK-NOT: error: .debug_line[0x{{[0-9a-f]+}}][0] has invalid file index 1 (valid values are [1,0]):
 --- !mach-o

rastogishubham added a commit to rastogishubham/llvm-project that referenced this pull request Jan 6, 2024
Accidentally didn't commit the review feedback before merging the PR

(cherry picked from commit f22dc88)
@dwblaikie
Copy link
Collaborator

(if you send a pull request, it's expected that review will be done - if you don't need review, please don't send a pull request (& commit directly instead) - so as not to confuse what needs review and what doesn't (& avoid the impression that something that did need review was then committed without that review being completed))

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Accidentally didn't commit the review feedback before merging the PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants