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. ================ Comment at: tools/lldb-vscode/VSCode.h:45 + +typedef std::map<uint32_t, SourceBreakpoint> SourceBreakpointMap; +typedef std::map<std::string, FunctionBreakpoint> FunctionBreakpointMap; ---------------- zturner wrote: > clayborg wrote: > > zturner wrote: > > > `llvm::DenseMap` > > Causes build errors when I tried switching. > What kind of build errors? `DenseMap<uint32_t, SourceBreakpoint>` shouldn't > cause any. If `SourceBreakpoint` was the key it might, but not if it's the > value. I meant to remove this. I have everything working with DenseMap, DenseSet and StringMap ================ Comment at: tools/lldb-vscode/VSCode.h:46 +typedef std::map<uint32_t, SourceBreakpoint> SourceBreakpointMap; +typedef std::map<std::string, FunctionBreakpoint> FunctionBreakpointMap; + ---------------- zturner wrote: > clayborg wrote: > > zturner wrote: > > > `llvm::StringMap` > > Doesn't work with std::string and we need the std::string to back the > > string content. > `llvm::StringMap` copies the keys so it will always be backed. Ditto, meant to remove this. Got everything working. ================ Comment at: tools/lldb-vscode/VSCode.h:61 + FILE *in; + FILE *out; + lldb::SBDebugger debugger; ---------------- This can work with a port number. Rather not switch away from FILE * for now. No real benefit. https://reviews.llvm.org/D50365 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
