mib marked 2 inline comments as done. mib added inline comments.
================ Comment at: lldb/source/Commands/CommandObjectFrame.cpp:880 auto func = - RegularExpressionSP(new RegularExpression(m_options.m_function)); + RegularExpressionSP(new RegularExpression(m_options.m_symbols.front())); StackFrameRecognizerManager::AddRecognizer(recognizer_sp, module, func); ---------------- jingham wrote: > labath wrote: > > mib wrote: > > > labath wrote: > > > > mib wrote: > > > > > labath wrote: > > > > > > Is there something which ensure that m_symbols contains at least > > > > > > one element here? (i.e., that we do not silently drop the extra > > > > > > symbols arguments) > > > > > Few lines above, we make sure the symbols list is not empty. > > > > Not empty is one thing, but what about the case when it contains 2 or > > > > more elements (`frame recognizer add --name foo --name bar --regex`)? > > > > Will that produce some kind of an error, or will it just ignore the > > > > second regex ? > > > It takes only the first symbol and ignores the other ones. > > Hmm.. that doesn't sound very useful. I think that should be an error. > Just put --name and --regex in different option groups when you define the > command. Then the command machinery won't allow you to specify both at the > same time (and the help will show they are mutually incompatible). > > I agree with Pavel, an error is what you want in that case. @jingham I think this is wrong because `--regex` is just a boolean flag, not the actual regular expression. When this flag is enabled, the overloading for `AddRecognizer`that is called is the one that holds `RegularExpressionSP`s instead of the overloading with a `ConstString` and `llvm::ArrayRef<ConstString>` ... I'll add the check above to make sure `m_symbols` has only 1 entry when the `--regex` flag is enable and return an error otherwise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76188/new/ https://reviews.llvm.org/D76188 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits