On Wed, Aug 15, 2018 at 4:59 PM Zachary Turner <ztur...@google.com> wrote:
> > > On Wed, Aug 15, 2018 at 4:48 PM Greg Clayton via Phabricator < > revi...@reviews.llvm.org> wrote: > >> clayborg marked 23 inline comments as done. >> clayborg added inline comments. >> >> >> ================ >> Comment at: tools/lldb-vscode/SourceBreakpoint.h:24 >> + // Set this breakpoint in LLDB as a new breakpoint >> + void SetBreakpoint(const char *source_path); >> +}; >> ---------------- >> zturner wrote: >> > clayborg wrote: >> > > zturner wrote: >> > > > `StringRef source_path` >> > > I am gonna leave this one as is because we must pass a "const char *" >> the API that is called inside the body of this method: >> > > >> > > ``` >> > > lldb::SBBreakpoint lldb::SBTarget::BreakpointCreateByLocation(const >> char *file, uint32_t line); >> > > ``` >> > > >> > In that case you can use `const llvm::Twine &`. You can pass many >> types of objects to it, such as `const char*`, `std::string`, >> `llvm::StringRef`. Then, inside the function you can write: >> > >> > ``` >> > llvm::SmallString<32> S; >> > StringRef NTS = source_path.toNullTerminatedStringRef(S); >> > ``` >> > >> > If the original parameter was constructed with a `const char*` or >> `std::string`, then `toNullTerminatedStringRef` is smart enough to know >> that it doesn't need to make a copy, and it just returns the pointer. If >> it was constructed with a `StringRef`, then it null terminates it using `S` >> as the backing storage and returns that pointer. So it's the best of both >> worlds >> That is just way too much work for no gain. > > > The gain is that const char* is unsafe and error prone and should be > avoided wherever possible unless it’s very cumbersome. > I forgot inflexible. As soon as a function takes a const char *, it starts trickling through the interface. Now callers of the function or other member variables become const char *. Then it goes out further. Eventually you end up with it being spread out throughout your whole program because of one function. I might be exaggerating slightly, but only slightly. The point is, Interface design should be orthogonal to implementation. It might be a minor nitpick, but we have a basically clean slate here so I would prefer to start on the right foot with modern c++ constructs and paradigms. >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
