[lldb-dev] Possible fix for Bug 21190 - CategoriesDataFormatterTestCase failure

[Todd Fiala]

I'll go ahead and track down the actual use case that was failing for him
originally.

I think in the meantime we just back out the original change and, if we
still need it, put it in without breaking other things.

-TOdd

On Thu, Oct 9, 2014 at 10:32 AM, Greg Clayton <gclayton at apple.com> wrote:

> Just a note for Tong: You can use single or double quotes to get around
> not being able to quote things:
>
> (lldb) expression --jit-args='-foo "Bar" -baz "123"'
>
> Not sure if that would help Tong's isssue.
>
> Greg
>
> > On Oct 8, 2014, at 7:09 PM, Todd Fiala <tfiala at google.com> wrote:
> >
> > Hey Alex,
> >
> > The option parsing quotes handling changes started when Tong was trying
> to get the expression parser compiler args setting introduced.  So I think
> the ultimate cleaner fix will end up needing to look at the compiler args
> he couldn't initially pass in, use that as a test case for a robust
> solution, and also use the issue you uncovered here as another case.
> >
> > I'd be in favor of a fix like what you have as a short-term stop-gap to
> address the immediate bug it is producing.  (Since you found a test case
> where it fails, this would be a great item to put a Python test around to
> protect against regressions).
> >
> > We can come back and clean this up more thoroughly as a lower-priority
> follow-up task.
> >
> > On Wed, Oct 8, 2014 at 6:25 PM, Alex Pepper <apepper at blueshiftinc.com>
> wrote:
> > So I found what was causing the bug and have a fix, but I don’t really
> like it.  Ideally we would do away with the hacky add+removal of quotes but
> that would take some additional time to find better solution.
> >
> > The problem occurs when you have two options with the same argument
> value string as in:
> >
> > type summary add Rectangle -s "Category1" -w Category1
> >
> > As it iterates through the options, the first time it removes the quotes
> on the first “Catagory” string by searching through the m_args list to
> match the argument that was returned from OptionsParser::Parse.  The
> problem arises when it searches for the next option value, it has already
> removed the quotes from the first “Category1” string so it matches again,
> then it checks if it was flagged for quote removal and removes the first
> character thinking it must be a quote.  The problem is there is no direct
> matching between the string returned from the OptionsParser::Parse and the
> list of args so I added in a last_argv_iter variable to track which args we
> have already checked.  This works ok but seems like it relies on the
> implementation of OptionsParser::Parse to always return options in order.
> A better way would be to remove all of the quote hackery but that will take
> some additional time to understand why it was added in the first place.
> >
> >
> > Here is the patch:
> >
> > diff --git a/source/Interpreter/Args.cpp b/source/Interpreter/Args.cpp
> > index 682feb9..4dd7383 100644
> > --- a/source/Interpreter/Args.cpp
> > +++ b/source/Interpreter/Args.cpp
> > @@ -661,6 +661,9 @@ Args::ParseOptions (Options &options)
> >          }
> >      }
> >
> > +    // Initialize the last argument iterator so we start checking at
> beginning of args list
> > +    int last_argv_iter = -1;
> > +
> >      while (1)
> >      {
> >          int long_options_index = -1;
> > @@ -717,6 +720,14 @@ Args::ParseOptions (Options &options)
> >                      argv_iter = 0;
> >                      for (auto args_iter = m_args.begin(); args_iter !=
> m_args.end(); args_iter++, argv_iter++)
> >                      {
> > +                        // Skip any args we have already checked from
> previous options
> > +                        if ( argv_iter <= last_argv_iter )
> > +                        {
> > +                            continue;
> > +                        }
> > +                        // Save the last argument we have checked
> > +                        last_argv_iter = argv_iter;
> > +
> >                          if (*args_iter == value &&
> GetArgumentQuoteCharAtIndex(argv_iter) != '\0')
> >                          {
> >                              *args_iter = args_iter->substr(1);
> >
> >
> > So in the short term we can add this in as a temporary fix unless
> someone has objections and we can look for a better solution longer term.
> >
> > - Alex Pepper
> >
> >
> >
> > --
> > Todd Fiala |   Software Engineer |     tfiala at google.com
> >
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
>
>


-- 
Todd Fiala | Software Engineer | tfiala at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20141009/228c57b8/attachment.html>