Skip to content

Conversation

@Michael137
Copy link
Member

These IsValid/SetValid APIs are only ever used from 1 data-formatter in the Swift LLDB fork. Since all the APIs on
SyntheticChildrenFrontEnd are meant to be overriden, there is no good way to enforce calling IsValid from the base. And we should just let that 1 data-formatter manage its own IsValid state.

These `IsValid`/`SetValid` APIs are only ever used from 1 data-formatter
in the Swift LLDB fork. Since all the APIs on
`SyntheticChildrenFrontEnd` are meant to be overriden, there is no good
way to enforce calling `IsValid` from the base. And we should just let
that 1 data-formatter manage its own `IsValid` state.
@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

These IsValid/SetValid APIs are only ever used from 1 data-formatter in the Swift LLDB fork. Since all the APIs on
SyntheticChildrenFrontEnd are meant to be overriden, there is no good way to enforce calling IsValid from the base. And we should just let that 1 data-formatter manage its own IsValid state.


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

1 Files Affected:

  • (modified) lldb/include/lldb/DataFormatters/TypeSynthetic.h (+1-6)
diff --git a/lldb/include/lldb/DataFormatters/TypeSynthetic.h b/lldb/include/lldb/DataFormatters/TypeSynthetic.h
index b147d66def730..4c57a80ee104e 100644
--- a/lldb/include/lldb/DataFormatters/TypeSynthetic.h
+++ b/lldb/include/lldb/DataFormatters/TypeSynthetic.h
@@ -28,13 +28,9 @@ class SyntheticChildrenFrontEnd {
 protected:
   ValueObject &m_backend;
 
-  void SetValid(bool valid) { m_valid = valid; }
-
-  bool IsValid() { return m_valid; }
-
 public:
   SyntheticChildrenFrontEnd(ValueObject &backend)
-      : m_backend(backend), m_valid(true) {}
+      : m_backend(backend) {}
 
   virtual ~SyntheticChildrenFrontEnd() = default;
 
@@ -100,7 +96,6 @@ class SyntheticChildrenFrontEnd {
                                                 CompilerType type);
 
 private:
-  bool m_valid;
   SyntheticChildrenFrontEnd(const SyntheticChildrenFrontEnd &) = delete;
   const SyntheticChildrenFrontEnd &
   operator=(const SyntheticChildrenFrontEnd &) = delete;

@github-actions
Copy link

github-actions bot commented Oct 20, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Oct 20, 2025
We removed the base-class `SyntheticChildrenFrontEnd::m_valid` member in
llvm#164249. The URL formatter is
the only user of this and should be able to keep track of its own
validity without going through the base.
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Oct 20, 2025
We removed the base-class `SyntheticChildrenFrontEnd::m_valid` member in llvm#164249. The URL formatter is the only user of this and should be able to keep track of its own validity without going through the base.
Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, IMO almost all IsValid APIs should be replaced by static constructor methods returning llvm::Expected anyway.

@Michael137 Michael137 merged commit 750d81a into llvm:main Oct 20, 2025
12 of 14 checks passed
@Michael137 Michael137 deleted the lldb/redundant-synthetic-frontend-valid branch October 20, 2025 23:54
Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025
…er (llvm#164249)

These `IsValid`/`SetValid` APIs are only ever used from 1 data-formatter
in the Swift LLDB fork. Since all the APIs on
`SyntheticChildrenFrontEnd` are meant to be overriden, there is no good
way to enforce calling `IsValid` from the base. And we should just let
that 1 data-formatter manage its own `IsValid` state.
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