[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.


================
Comment at: lldb/source/API/SBWatchpoint.cpp:319
+  }
+  return false;
+}
----------------
delcypher wrote:
> @bulbazord What I don't like about my implementation here is that the user can't tell the difference between
> 
> 1. there's no shared pointer
> AND
> 2. watchpoint is an expression watchpoint.
> 
> Is 1, common? Feels like this function should really return a `std:optional<bool>` but I'm not sure how to make that play well with SWIG.
I'm not sure if 1 is common. To distinguish between 1 and 2, clients will also want to also check the validity of the watchpoint with `IsValid` or `operator bool` since those just get the shared pointer and see if its valid. We could make a std::optional work through a swig type map but I'm not sure I like the idea of a yes-or-no-question API being able to return 'None'. If it's not a watch variable, then 'false' makes sense for an invalid watchpoint.

Another option could be to change the interface a bit. Maybe create an enum called WatchpointKind containing 3 values, "Variable", "Expression", and "Invalid". You could then change this method to be `GetWatchpointKind` and return the right value depending on a combination of `GetSP()` and `IsWatchVariable`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144937/new/

https://reviews.llvm.org/D144937