jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed.
No good deed goes unpunished... You made ParseCanonicalReference more beautiful but forgot to update the header doc. Also I didn't see the comment for SplitIDRangeExpression. > BreakpointID.h:74-75 > //------------------------------------------------------------------ > - static bool ParseCanonicalReference(const char *input, > - lldb::break_id_t *break_id, > - lldb::break_id_t *break_loc_id); > + static llvm::Optional<BreakpointID> > + ParseCanonicalReference(llvm::StringRef input); > You changed the interface but not the header doc for it. > BreakpointIDList.cpp:329-330 > > -bool BreakpointIDList::StringContainsIDRangeExpression(const char *in_string, > - size_t > *range_start_len, > - size_t > *range_end_pos) { > - bool is_range_expression = false; > - std::string arg_str = in_string; > - std::string::size_type idx; > - std::string::size_type start_pos = 0; > - > - *range_start_len = 0; > - *range_end_pos = 0; > - > - int specifiers_size = 0; > - for (int i = 0; BreakpointID::g_range_specifiers[i] != nullptr; ++i) > - ++specifiers_size; > - > - for (int i = 0; i < specifiers_size && !is_range_expression; ++i) { > - const char *specifier_str = BreakpointID::g_range_specifiers[i]; > - size_t len = strlen(specifier_str); > - idx = arg_str.find(BreakpointID::g_range_specifiers[i]); > - if (idx != std::string::npos) { > - *range_start_len = idx - start_pos; > - std::string start_str = arg_str.substr(start_pos, *range_start_len); > - if (idx + len < arg_str.length()) { > - *range_end_pos = idx + len; > - std::string end_str = arg_str.substr(*range_end_pos); > - if (BreakpointID::IsValidIDExpression(start_str.c_str()) && > - BreakpointID::IsValidIDExpression(end_str.c_str())) { > - is_range_expression = true; > - //*range_start = start_str; > - //*range_end = end_str; > - } > - } > - } > - } > +std::pair<llvm::StringRef, llvm::StringRef> > +BreakpointIDList::SplitIDRangeExpression(llvm::StringRef in_string) { > + for (auto specifier_str : BreakpointID::GetRangeSpecifiers()) { Did you upload the latest version of your patch, I don't see a comment here... https://reviews.llvm.org/D25158 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits