Skip to content

[lldb] Add comment on cross printing of summary/value #81681

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

Conversation

kastiglione
Copy link
Contributor

@kastiglione kastiglione commented Feb 13, 2024

Adds a comment to indicate intention of a piece of value printing code.

I was initially surprised to see this code (distilled for emphasis):

if (str.empty()) {
  if (style == eValueObjectRepresentationStyleValue)
    str = GetSummaryAsCString();
  else if (style == eValueObjectRepresentationStyleSummary)
    str = GetValueAsCString();
}

My first thought was "is this a bug?", but I realized it was likely intentional. This change adds a comment to indicate yes, this is intentional.

@llvmbot
Copy link
Member

llvmbot commented Feb 13, 2024

@llvm/pr-subscribers-lldb

Author: Dave Lee (kastiglione)

Changes

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

1 Files Affected:

  • (modified) lldb/source/Core/ValueObject.cpp (+1)
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index e80042826f7d64..2a2bb128d33c57 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1312,6 +1312,7 @@ bool ValueObject::DumpPrintableRepresentation(
       break;
     }
 
+    // Try summary if no value, and value if no summary.
     if (str.empty()) {
       if (val_obj_display == eValueObjectRepresentationStyleValue)
         str = GetSummaryAsCString();

@jimingham
Copy link
Collaborator

That did deserve a comment. To be pedantic, in the second branch you try value/location if no summary, but what you have is fine.

@kastiglione
Copy link
Contributor Author

I'll fine tune the wording to match the code.

@kastiglione kastiglione merged commit 1da4494 into llvm:main Feb 14, 2024
@kastiglione kastiglione deleted the lldb-Add-comment-on-cross-printing-of-summary-value branch February 14, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants