[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

[Jim Ingham via Phabricator via lldb-commits]

jingham 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);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76188