diff mbox series

[1/6] Rename private member of FrameDecorator

Message ID 20230725-dap-bt-path-v1-1-bb015b0d8e54@adacore.com
State New
Headers
Series Several DAP fixes for VSCode |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey July 25, 2023, 1:49 p.m. UTC 
  In Python, a member name starting with "__" is specially handled to
make it private to the class.  This patch ensures that this is done
for the private method of FrameDecorator.
---
 gdb/python/lib/gdb/FrameDecorator.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Lancelot SIX July 28, 2023, 2:50 p.m. UTC | #1 
On Tue, Jul 25, 2023 at 07:49:38AM -0600, Tom Tromey via Gdb-patches wrote:
> In Python, a member name starting with "__" is specially handled to
> make it private to the class.  This patch ensures that this is done
> for the private method of FrameDecorator.

Hi,

My understanding is that the "__" prefix is more a syntactic sugar to
avoid name clashes with subclasses than making it strictly private[1].

In this case, the method still exists as
FrameDecorator._FrameDecorator__is_limited_frame, which can be accessed
from outside of FrameDecorator.  I agree it can be less intuitive to
access this method from a subclass as one would need to call
self._FrameDecorator__is_limited_frame, which is probably what you are
looking for.

I do not object to this change, I am just noting that nothing is really
private in Python, and it seems to me that the commit message promises
otherwise.

Best,
Lancelot.

[1] https://docs.python.org/3/tutorial/classes.html#private-variables

> ---
>  gdb/python/lib/gdb/FrameDecorator.py | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
> index 050cb934b7c..c4a7806e434 100644
> --- a/gdb/python/lib/gdb/FrameDecorator.py
> +++ b/gdb/python/lib/gdb/FrameDecorator.py
> @@ -53,7 +53,7 @@ class FrameDecorator(object):
>          self._base = base
>  
>      @staticmethod
> -    def _is_limited_frame(frame):
> +    def __is_limited_frame(frame):
>          """Internal utility to determine if the frame is special or
>          limited."""
>          sal = frame.find_sal()
> @@ -148,7 +148,7 @@ class FrameDecorator(object):
>              return self._base.frame_args()
>  
>          frame = self.inferior_frame()
> -        if self._is_limited_frame(frame):
> +        if self.__is_limited_frame(frame):
>              return None
>  
>          args = FrameVars(frame)
> @@ -164,7 +164,7 @@ class FrameDecorator(object):
>              return self._base.frame_locals()
>  
>          frame = self.inferior_frame()
> -        if self._is_limited_frame(frame):
> +        if self.__is_limited_frame(frame):
>              return None
>  
>          args = FrameVars(frame)
> @@ -179,7 +179,7 @@ class FrameDecorator(object):
>              return self._base.line()
>  
>          frame = self.inferior_frame()
> -        if self._is_limited_frame(frame):
> +        if self.__is_limited_frame(frame):
>              return None
>  
>          sal = frame.find_sal()
> 
> -- 
> 2.40.1
>
Tom Tromey July 31, 2023, 2:30 p.m. UTC | #2 
>>>>> "Lancelot" == Lancelot SIX <lsix@lancelotsix.com> writes:

Lancelot> I do not object to this change, I am just noting that nothing is really
Lancelot> private in Python, and it seems to me that the commit message promises
Lancelot> otherwise.

I've reworded it to:

    In Python, a member name starting with "__" is specially handled to
    make it "more private" to the class -- it isn't truly private, but it
    is renamed to make it less likely to be reused by mistake.

Tom
Lancelot SIX July 31, 2023, 2:36 p.m. UTC | #3 
On Mon, Jul 31, 2023 at 08:30:05AM -0600, Tom Tromey wrote:
> >>>>> "Lancelot" == Lancelot SIX <lsix@lancelotsix.com> writes:
> 
> Lancelot> I do not object to this change, I am just noting that nothing is really
> Lancelot> private in Python, and it seems to me that the commit message promises
> Lancelot> otherwise.
> 
> I've reworded it to:
> 
>     In Python, a member name starting with "__" is specially handled to
>     make it "more private" to the class -- it isn't truly private, but it
>     is renamed to make it less likely to be reused by mistake.
> 
> Tom

Hi,

FWIW, this looks good to me.

Best,
Lancelot.
diff mbox series

Patch

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index 050cb934b7c..c4a7806e434 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -53,7 +53,7 @@  class FrameDecorator(object):
         self._base = base
 
     @staticmethod
-    def _is_limited_frame(frame):
+    def __is_limited_frame(frame):
         """Internal utility to determine if the frame is special or
         limited."""
         sal = frame.find_sal()
@@ -148,7 +148,7 @@  class FrameDecorator(object):
             return self._base.frame_args()
 
         frame = self.inferior_frame()
-        if self._is_limited_frame(frame):
+        if self.__is_limited_frame(frame):
             return None
 
         args = FrameVars(frame)
@@ -164,7 +164,7 @@  class FrameDecorator(object):
             return self._base.frame_locals()
 
         frame = self.inferior_frame()
-        if self._is_limited_frame(frame):
+        if self.__is_limited_frame(frame):
             return None
 
         args = FrameVars(frame)
@@ -179,7 +179,7 @@  class FrameDecorator(object):
             return self._base.line()
 
         frame = self.inferior_frame()
-        if self._is_limited_frame(frame):
+        if self.__is_limited_frame(frame):
             return None
 
         sal = frame.find_sal()