Skip to content

Conversation

felipepiovezan
Copy link
Contributor

@felipepiovezan felipepiovezan commented Sep 8, 2025

Some comments were "suffixed" to member variable declarations; these are moved to before the variable.

Some constructors and operators were just defaulted and not necessary.

Some comments dividing the class into logical sections, like "// constructors and destructors", were not applied everywhere. These were removed. They are used in some parts of LLDB, but are the exception.

An include was not needed.

The operator != can be defined in terms of ==.

@llvmbot
Copy link
Member

llvmbot commented Sep 8, 2025

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

Some comments were "suffixed" to member variable declarations; these are moved to before the variable.

Some constructors and operators were just defaulted and not necessary.

Some comments dividing the class into logical sections, like "// constructors and destructors", were not applied everywhere. These were removed. They are used in some parts of LLDB, but are the exception.

An include was not needed.


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

1 Files Affected:

  • (modified) lldb/include/lldb/Target/StackID.h (+16-34)
diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h
index fddbc8e48dfdc..4b49964853f7b 100644
--- a/lldb/include/lldb/Target/StackID.h
+++ b/lldb/include/lldb/Target/StackID.h
@@ -10,7 +10,6 @@
 #define LLDB_TARGET_STACKID_H
 
 #include "lldb/Core/AddressRange.h"
-#include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
@@ -18,15 +17,11 @@ class Process;
 
 class StackID {
 public:
-  // Constructors and Destructors
   StackID() = default;
 
   explicit StackID(lldb::addr_t pc, lldb::addr_t cfa,
                    SymbolContextScope *symbol_scope, Process *process);
 
-  StackID(const StackID &rhs)
-      : m_pc(rhs.m_pc), m_cfa(rhs.m_cfa), m_symbol_scope(rhs.m_symbol_scope) {}
-
   ~StackID() = default;
 
   lldb::addr_t GetPC() const { return m_pc; }
@@ -51,41 +46,28 @@ class StackID {
 
   void Dump(Stream *s);
 
-  // Operators
-  const StackID &operator=(const StackID &rhs) {
-    if (this != &rhs) {
-      m_pc = rhs.m_pc;
-      m_cfa = rhs.m_cfa;
-      m_symbol_scope = rhs.m_symbol_scope;
-    }
-    return *this;
-  }
-
 protected:
   friend class StackFrame;
 
   void SetPC(lldb::addr_t pc, Process *process);
   void SetCFA(lldb::addr_t cfa, Process *process);
 
-  lldb::addr_t m_pc =
-      LLDB_INVALID_ADDRESS; // The pc value for the function/symbol for this
-                            // frame. This will
-  // only get used if the symbol scope is nullptr (the code where we are
-  // stopped is not represented by any function or symbol in any shared
-  // library).
-  lldb::addr_t m_cfa =
-      LLDB_INVALID_ADDRESS; // The call frame address (stack pointer) value
-                            // at the beginning of the function that uniquely
-                            // identifies this frame (along with m_symbol_scope
-                            // below)
-  SymbolContextScope *m_symbol_scope =
-      nullptr; // If nullptr, there is no block or symbol for this frame.
-               // If not nullptr, this will either be the scope for the
-               // lexical block for the frame, or the scope for the
-               // symbol. Symbol context scopes are always be unique
-               // pointers since the are part of the Block and Symbol
-               // objects and can easily be used to tell if a stack ID
-               // is the same as another.
+  // The pc value for the function/symbol for this frame. This will only get
+  // used if the symbol scope is nullptr (the code where we are stopped is not
+  // represented by any function or symbol in any shared library).
+  lldb::addr_t m_pc = LLDB_INVALID_ADDRESS;
+
+  // The call frame address (stack pointer) value at the beginning of the
+  // function that uniquely identifies this frame (along with m_symbol_scope
+  // below)
+  lldb::addr_t m_cfa = LLDB_INVALID_ADDRESS;
+
+  // If nullptr, there is no block or symbol for this frame. If not nullptr,
+  // this will either be the scope for the lexical block for the frame, or the
+  // scope for the symbol. Symbol context scopes are always be unique pointers
+  // since the are part of the Block and Symbol objects and can easily be used
+  // to tell if a stack ID is the same as another.
+  SymbolContextScope *m_symbol_scope = nullptr;
 };
 
 bool operator==(const StackID &lhs, const StackID &rhs);

Some comments were "suffixed" to member variable declarations; these are
moved to before the variable.

Some constructors and operators were just defaulted and not necessary.

Some comments dividing the class into logical sections, like "//
constructors and destructors", were not applied everywhere. These were
removed. They are used in some parts of LLDB, but are the exception.

An include was not needed.

The operator != can be defined in terms of ==.
// pointers since the are part of the Block and Symbol
// objects and can easily be used to tell if a stack ID
// is the same as another.
// The pc value for the function/symbol for this frame. This will only get
Copy link
Member

Choose a reason for hiding this comment

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

We could turn these into doxygen comments by making them ///

Copy link
Member

@Michael137 Michael137 left a comment

Choose a reason for hiding this comment

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

LGTM

@felipepiovezan felipepiovezan merged commit 54b3dc1 into llvm:main Sep 8, 2025
9 checks passed
@felipepiovezan felipepiovezan deleted the felipe/stack_id_nfcs branch September 8, 2025 15:47
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Sep 12, 2025
Some comments were "suffixed" to member variable declarations; these are
moved to before the variable.

Some constructors and operators were just defaulted and not necessary.

Some comments dividing the class into logical sections, like "//
constructors and destructors", were not applied everywhere. These were
removed. They are used in some parts of LLDB, but are the exception.

An include was not needed.

The operator != can be defined in terms of ==.

(cherry picked from commit 54b3dc1)
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Sep 24, 2025
Some comments were "suffixed" to member variable declarations; these are
moved to before the variable.

Some constructors and operators were just defaulted and not necessary.

Some comments dividing the class into logical sections, like "//
constructors and destructors", were not applied everywhere. These were
removed. They are used in some parts of LLDB, but are the exception.

An include was not needed.

The operator != can be defined in terms of ==.

(cherry picked from commit 54b3dc1)
(cherry picked from commit 8cc7914)
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