================
@@ -0,0 +1,66 @@
+//===-- RealpathPrefixes.cpp 
----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Utility/RealpathPrefixes.h"
+
+#include "lldb/Target/Statistics.h"
+#include "lldb/Target/Target.h"
----------------
labath wrote:

> What is the general guidance of when a class should be in "Target" vs. in 
> "Utility"?

We have a description [here](https://lldb.llvm.org/resources/overview.html), 
though it leaves a lot of room for interpretation. I think it makes sense for 
this to be in Utility, but then it should not depend on Target, so that it can 
be an actual utility that can be reused for more than one purpose.

> 
> > dependency inversion technique
> 
> Like how? Below is my attempt. Haven't updated the patch. Want to get your 
> feedback first by describing it below.
> 
> I tried to make an interface `RealpathPrefixStats` which contains the two 
> increment functions, then have `TargetStats` implement this interface (thus 
> virtual functions), in the hope of the Target can create the `RealpathPrefix` 
> and feed its own `TargetStats` to it as `RealpathPrefixStats` (where 
> `RealpathPrefix` only sees `RealpathPrefixStats`, and both are in `Utility`).
> 
> However, there is the lifecycle problem as Jim pointed out earlier, that what 
> guarantees that the `RealpathPrefixStats` object lives longer than the 
> `RealpathPrefix` object. In order to guarantee this, I will need to make 
> `Target::m_stats` a shared pointer, then give a weak pointer of 
> `RealpathPrefixStats` to `RealpathPrefix`.
> 
> Is the above a good plan? Do you see a better approach?

Maybe (to both questions).

You may not need to make m_stats a shared pointer if you use the 
little-known-but-very powerful feature of shared_ptr -- the [aliasing 
constructor](https://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr). 
With it, you can create a shared_ptr, which points to a subfield (m_stats), but 
keeps the whole object alive like so `std::shared_ptr(shared_from_this(), 
&m_stats)`.

A different approach would be to implement the interfaces you need on the 
Target object itself.

It may also be possible to keep a record of the statistics on the inside the 
object itself, and only later add it to the ones in the Target. This would make 
sense if there's a natural place where you can insert the call to gather the 
stats (either when the object is destructed, when the stats are requested, 
etc.).

https://github.com/llvm/llvm-project/pull/102223
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to