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

Reply via email to