[Lldb-commits] [lldb] r287354 - Resubmit "Remove an output-parameter from Variable function".

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Fri Nov 18 12:32:19 PST 2016


I admit I've been moving somewhat fast with these changes.  The parameter
in question was only used in one logging statement, so the benefit did not
seem worth the added cost of complexity.  But you're right, that involved a
judgement call on my part that I didn't vet first.

+Enrico Granata <egranata at apple.com> , who can hopefully do a post-commit
review on this.  I can add it back without much effort if he feels it's
important.

On Fri, Nov 18, 2016 at 12:02 PM Jim Ingham <jingham at apple.com> wrote:

> I didn’t see this patch go up for review.  The parameter you removed might
> have been vestigial or might have been one that the owner of this code was
> planning to do something with.  Anyway, this seems to go beyond purely
> formal changes and so should have been discussed.  My apologies if I just
> missed the review.
>
> Jim
>
> > On Nov 18, 2016, at 9:55 AM, Zachary Turner via lldb-commits <
> lldb-commits at lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Fri Nov 18 11:55:04 2016
> > New Revision: 287354
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=287354&view=rev
> > Log:
> > Resubmit "Remove an output-parameter from Variable function".
> >
> > The scanning algorithm had a few little subtleties that I
> > overlooked, but this patch should fix everything.
> >
> > I still haven't changed the function to take a StringRef since
> > that has some trickle down effect and is mostly mechanical,
> > I just wanted to get the tricky part as isolated as possible.
> >
> > Modified:
> >    lldb/trunk/include/lldb/Core/ValueObject.h
> >    lldb/trunk/source/Core/FormatEntity.cpp
> >    lldb/trunk/source/Core/ValueObject.cpp
> >    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
> >    lldb/trunk/source/Symbol/Variable.cpp
> >
> > Modified: lldb/trunk/include/lldb/Core/ValueObject.h
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObject.h?rev=287354&r1=287353&r2=287354&view=diff
> >
> ==============================================================================
> > --- lldb/trunk/include/lldb/Core/ValueObject.h (original)
> > +++ lldb/trunk/include/lldb/Core/ValueObject.h Fri Nov 18 11:55:04 2016
> > @@ -394,7 +394,7 @@ public:
> >       GetExpressionPathFormat =
> eGetExpressionPathFormatDereferencePointers);
> >
> >   lldb::ValueObjectSP GetValueForExpressionPath(
> > -      const char *expression, const char **first_unparsed = nullptr,
> > +      const char *expression,
> >       ExpressionPathScanEndReason *reason_to_stop = nullptr,
> >       ExpressionPathEndResultType *final_value_type = nullptr,
> >       const GetValueForExpressionPathOptions &options =
> > @@ -1002,8 +1002,7 @@ private:
> >   virtual CompilerType MaybeCalculateCompleteType();
> >
> >   lldb::ValueObjectSP GetValueForExpressionPath_Impl(
> > -      const char *expression_cstr, const char **first_unparsed,
> > -      ExpressionPathScanEndReason *reason_to_stop,
> > +      const char *expression_cstr, ExpressionPathScanEndReason
> *reason_to_stop,
> >       ExpressionPathEndResultType *final_value_type,
> >       const GetValueForExpressionPathOptions &options,
> >       ExpressionPathAftermath *final_task_on_target);
> >
> > Modified: lldb/trunk/source/Core/FormatEntity.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FormatEntity.cpp?rev=287354&r1=287353&r2=287354&view=diff
> >
> ==============================================================================
> > --- lldb/trunk/source/Core/FormatEntity.cpp (original)
> > +++ lldb/trunk/source/Core/FormatEntity.cpp Fri Nov 18 11:55:04 2016
> > @@ -614,7 +614,6 @@ static ValueObjectSP ExpandIndexedExpres
> >   if (log)
> >     log->Printf("[ExpandIndexedExpression] name to deref: %s",
> >                 ptr_deref_buffer.c_str());
> > -  const char *first_unparsed;
> >   ValueObject::GetValueForExpressionPathOptions options;
> >   ValueObject::ExpressionPathEndResultType final_value_type;
> >   ValueObject::ExpressionPathScanEndReason reason_to_stop;
> > @@ -622,20 +621,18 @@ static ValueObjectSP ExpandIndexedExpres
> >       (deref_pointer ? ValueObject::eExpressionPathAftermathDereference
> >                      : ValueObject::eExpressionPathAftermathNothing);
> >   ValueObjectSP item = valobj->GetValueForExpressionPath(
> > -      ptr_deref_buffer.c_str(), &first_unparsed, &reason_to_stop,
> > -      &final_value_type, options, &what_next);
> > +      ptr_deref_buffer.c_str(), &reason_to_stop, &final_value_type,
> options,
> > +      &what_next);
> >   if (!item) {
> >     if (log)
> > -      log->Printf("[ExpandIndexedExpression] ERROR: unparsed portion =
> %s, why "
> > -                  "stopping = %d,"
> > +      log->Printf("[ExpandIndexedExpression] ERROR: why stopping = %d,"
> >                   " final_value_type %d",
> > -                  first_unparsed, reason_to_stop, final_value_type);
> > +                  reason_to_stop, final_value_type);
> >   } else {
> >     if (log)
> > -      log->Printf("[ExpandIndexedExpression] ALL RIGHT: unparsed
> portion = %s, "
> > -                  "why stopping = %d,"
> > +      log->Printf("[ExpandIndexedExpression] ALL RIGHT: why stopping =
> %d,"
> >                   " final_value_type %d",
> > -                  first_unparsed, reason_to_stop, final_value_type);
> > +                  reason_to_stop, final_value_type);
> >   }
> >   return item;
> > }
> > @@ -724,7 +721,6 @@ static bool DumpValue(Stream &s, const S
> >   int64_t index_lower = -1;
> >   int64_t index_higher = -1;
> >   bool is_array_range = false;
> > -  const char *first_unparsed;
> >   bool was_plain_var = false;
> >   bool was_var_format = false;
> >   bool was_var_indexed = false;
> > @@ -762,25 +758,23 @@ static bool DumpValue(Stream &s, const S
> >       log->Printf("[Debugger::FormatPrompt] symbol to expand: %s",
> >                   expr_path.c_str());
> >
> > -    target = valobj
> > -                 ->GetValueForExpressionPath(expr_path.c_str(),
> &first_unparsed,
> > -                                             &reason_to_stop,
> &final_value_type,
> > -                                             options, &what_next)
> > -                 .get();
> > +    target =
> > +        valobj
> > +            ->GetValueForExpressionPath(expr_path.c_str(),
> &reason_to_stop,
> > +                                        &final_value_type, options,
> &what_next)
> > +            .get();
> >
> >     if (!target) {
> >       if (log)
> > -        log->Printf("[Debugger::FormatPrompt] ERROR: unparsed portion =
> %s, "
> > -                    "why stopping = %d,"
> > +        log->Printf("[Debugger::FormatPrompt] ERROR: why stopping = %d,"
> >                     " final_value_type %d",
> > -                    first_unparsed, reason_to_stop, final_value_type);
> > +                    reason_to_stop, final_value_type);
> >       return false;
> >     } else {
> >       if (log)
> > -        log->Printf("[Debugger::FormatPrompt] ALL RIGHT: unparsed
> portion = "
> > -                    "%s, why stopping = %d,"
> > +        log->Printf("[Debugger::FormatPrompt] ALL RIGHT: why stopping =
> %d,"
> >                     " final_value_type %d",
> > -                    first_unparsed, reason_to_stop, final_value_type);
> > +                    reason_to_stop, final_value_type);
> >       target = target
> >                    ->GetQualifiedRepresentationIfAvailable(
> >                        target->GetDynamicValueType(), true)
> >
> > Modified: lldb/trunk/source/Core/ValueObject.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=287354&r1=287353&r2=287354&view=diff
> >
> ==============================================================================
> > --- lldb/trunk/source/Core/ValueObject.cpp (original)
> > +++ lldb/trunk/source/Core/ValueObject.cpp Fri Nov 18 11:55:04 2016
> > @@ -1922,7 +1922,7 @@ ValueObject::GetSyntheticExpressionPathC
> >     // We haven't made a synthetic array member for expression yet, so
> >     // lets make one and cache it for any future reference.
> >     synthetic_child_sp = GetValueForExpressionPath(
> > -        expression, NULL, NULL, NULL,
> > +        expression, NULL, NULL,
> >         GetValueForExpressionPathOptions().SetSyntheticChildrenTraversal(
> >
>  GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
> >                 None));
> > @@ -2166,13 +2166,11 @@ void ValueObject::GetExpressionPath(Stre
> > }
> >
> > ValueObjectSP ValueObject::GetValueForExpressionPath(
> > -    const char *expression, const char **first_unparsed,
> > -    ExpressionPathScanEndReason *reason_to_stop,
> > +    const char *expression, ExpressionPathScanEndReason *reason_to_stop,
> >     ExpressionPathEndResultType *final_value_type,
> >     const GetValueForExpressionPathOptions &options,
> >     ExpressionPathAftermath *final_task_on_target) {
> >
> > -  const char *dummy_first_unparsed;
> >   ExpressionPathScanEndReason dummy_reason_to_stop =
> >       ValueObject::eExpressionPathScanEndReasonUnknown;
> >   ExpressionPathEndResultType dummy_final_value_type =
> > @@ -2181,8 +2179,7 @@ ValueObjectSP ValueObject::GetValueForEx
> >       ValueObject::eExpressionPathAftermathNothing;
> >
> >   ValueObjectSP ret_val = GetValueForExpressionPath_Impl(
> > -      expression, first_unparsed ? first_unparsed :
> &dummy_first_unparsed,
> > -      reason_to_stop ? reason_to_stop : &dummy_reason_to_stop,
> > +      expression, reason_to_stop ? reason_to_stop :
> &dummy_reason_to_stop,
> >       final_value_type ? final_value_type : &dummy_final_value_type,
> options,
> >       final_task_on_target ? final_task_on_target
> >                            : &dummy_final_task_on_target);
> > @@ -2237,8 +2234,7 @@ ValueObjectSP ValueObject::GetValueForEx
> > }
> >
> > ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
> > -    const char *expression_cstr, const char **first_unparsed,
> > -    ExpressionPathScanEndReason *reason_to_stop,
> > +    const char *expression_cstr2, ExpressionPathScanEndReason
> *reason_to_stop,
> >     ExpressionPathEndResultType *final_result,
> >     const GetValueForExpressionPathOptions &options,
> >     ExpressionPathAftermath *what_next) {
> > @@ -2247,12 +2243,10 @@ ValueObjectSP ValueObject::GetValueForEx
> >   if (!root.get())
> >     return ValueObjectSP();
> >
> > -  *first_unparsed = expression_cstr;
> > +  llvm::StringRef remainder(expression_cstr2);
> >
> >   while (true) {
> > -
> > -    const char *expression_cstr =
> > -        *first_unparsed; // hide the top level expression_cstr
> > +    llvm::StringRef temp_expression = remainder;
> >
> >     CompilerType root_compiler_type = root->GetCompilerType();
> >     CompilerType pointee_compiler_type;
> > @@ -2263,20 +2257,20 @@ ValueObjectSP ValueObject::GetValueForEx
> >     if (pointee_compiler_type)
> >
>  pointee_compiler_type_info.Reset(pointee_compiler_type.GetTypeInfo());
> >
> > -    if (!expression_cstr || *expression_cstr == '\0') {
> > +    if (temp_expression.empty()) {
> >       *reason_to_stop =
> ValueObject::eExpressionPathScanEndReasonEndOfString;
> >       return root;
> >     }
> >
> > -    switch (*expression_cstr) {
> > +    switch (temp_expression.front()) {
> >     case '-': {
> > +      temp_expression = temp_expression.drop_front();
> >       if (options.m_check_dot_vs_arrow_syntax &&
> >           root_compiler_type_info.Test(eTypeIsPointer)) // if you are
> trying to
> >                                                         // use -> on a
> >                                                         // non-pointer
> and I
> >                                                         // must catch
> the error
> >       {
> > -        *first_unparsed = expression_cstr;
> >         *reason_to_stop =
> >             ValueObject::eExpressionPathScanEndReasonArrowInsteadOfDot;
> >         *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
> > @@ -2287,48 +2281,46 @@ ValueObjectSP ValueObject::GetValueForEx
> >                                                        // when this is
> forbidden
> >           root_compiler_type_info.Test(eTypeIsPointer) &&
> >           options.m_no_fragile_ivar) {
> > -        *first_unparsed = expression_cstr;
> >         *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonFragileIVarNotAllowed;
> >         *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
> >         return ValueObjectSP();
> >       }
> > -      if (expression_cstr[1] != '>') {
> > -        *first_unparsed = expression_cstr;
> > +      if (!temp_expression.startswith(">")) {
> >         *reason_to_stop =
> >             ValueObject::eExpressionPathScanEndReasonUnexpectedSymbol;
> >         *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
> >         return ValueObjectSP();
> >       }
> > -      expression_cstr++; // skip the -
> >     }
> >       LLVM_FALLTHROUGH;
> >     case '.': // or fallthrough from ->
> >     {
> > -      if (options.m_check_dot_vs_arrow_syntax && *expression_cstr ==
> '.' &&
> > +      if (options.m_check_dot_vs_arrow_syntax &&
> > +          temp_expression.front() == '.' &&
> >           root_compiler_type_info.Test(eTypeIsPointer)) // if you are
> trying to
> >                                                         // use . on a
> pointer
> >                                                         // and I must
> catch the
> >                                                         // error
> >       {
> > -        *first_unparsed = expression_cstr;
> >         *reason_to_stop =
> >             ValueObject::eExpressionPathScanEndReasonDotInsteadOfArrow;
> >         *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
> > -        return ValueObjectSP();
> > +        return nullptr;
> >       }
> > -      expression_cstr++; // skip .
> > -      const char *next_separator = strpbrk(expression_cstr + 1, "-.[");
> > +      temp_expression = temp_expression.drop_front(); // skip . or >
> > +
> > +      size_t next_sep_pos = temp_expression.find_first_of("-.[", 1);
> >       ConstString child_name;
> > -      if (!next_separator) // if no other separator just expand this
> last layer
> > +      if (next_sep_pos == llvm::StringRef::npos) // if no other
> separator just
> > +                                                 // expand this last
> layer
> >       {
> > -        child_name.SetCString(expression_cstr);
> > +        child_name.SetString(temp_expression);
> >         ValueObjectSP child_valobj_sp =
> >             root->GetChildMemberWithName(child_name, true);
> >
> >         if (child_valobj_sp.get()) // we know we are done, so just return
> >         {
> > -          *first_unparsed = "";
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonEndOfString;
> >           *final_result = ValueObject::eExpressionPathEndResultTypePlain;
> > @@ -2378,28 +2370,28 @@ ValueObjectSP ValueObject::GetValueForEx
> >         // so we hit the "else" branch, and return an error
> >         if (child_valobj_sp.get()) // if it worked, just return
> >         {
> > -          *first_unparsed = "";
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonEndOfString;
> >           *final_result = ValueObject::eExpressionPathEndResultTypePlain;
> >           return child_valobj_sp;
> >         } else {
> > -          *first_unparsed = expression_cstr;
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > +          return nullptr;
> >         }
> >       } else // other layers do expand
> >       {
> > -        child_name.SetCStringWithLength(expression_cstr,
> > -                                        next_separator -
> expression_cstr);
> > +        llvm::StringRef next_separator =
> temp_expression.substr(next_sep_pos);
> > +
> > +        child_name.SetString(temp_expression.slice(0, next_sep_pos));
> > +
> >         ValueObjectSP child_valobj_sp =
> >             root->GetChildMemberWithName(child_name, true);
> >         if (child_valobj_sp.get()) // store the new root and move on
> >         {
> >           root = child_valobj_sp;
> > -          *first_unparsed = next_separator;
> > +          remainder = next_separator;
> >           *final_result = ValueObject::eExpressionPathEndResultTypePlain;
> >           continue;
> >         } else {
> > @@ -2448,15 +2440,14 @@ ValueObjectSP ValueObject::GetValueForEx
> >         if (child_valobj_sp.get()) // if it worked, move on
> >         {
> >           root = child_valobj_sp;
> > -          *first_unparsed = next_separator;
> > +          remainder = next_separator;
> >           *final_result = ValueObject::eExpressionPathEndResultTypePlain;
> >           continue;
> >         } else {
> > -          *first_unparsed = expression_cstr;
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > +          return nullptr;
> >         }
> >       }
> >       break;
> > @@ -2474,7 +2465,6 @@ ValueObjectSP ValueObject::GetValueForEx
> >
>  GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
> >                   None) // ...only chance left is synthetic
> >           {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonRangeOperatorInvalid;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > @@ -2484,26 +2474,23 @@ ValueObjectSP ValueObject::GetValueForEx
> >                                                       // check that we
> can
> >                                                       // expand bitfields
> >         {
> > -          *first_unparsed = expression_cstr;
> >           *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonRangeOperatorNotAllowed;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> >           return ValueObjectSP();
> >         }
> >       }
> > -      if (*(expression_cstr + 1) ==
> > +      if (temp_expression[1] ==
> >           ']') // if this is an unbounded range it only works for arrays
> >       {
> >         if (!root_compiler_type_info.Test(eTypeIsArray)) {
> > -          *first_unparsed = expression_cstr;
> >           *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonEmptyRangeNotAllowed;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > +          return nullptr;
> >         } else // even if something follows, we cannot expand unbounded
> ranges,
> >                // just let the caller do it
> >         {
> > -          *first_unparsed = expression_cstr + 2;
> >           *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonArrayRangeOperatorMet;
> >           *final_result =
> > @@ -2511,32 +2498,36 @@ ValueObjectSP ValueObject::GetValueForEx
> >           return root;
> >         }
> >       }
> > -      const char *separator_position = ::strchr(expression_cstr + 1,
> '-');
> > -      const char *close_bracket_position = ::strchr(expression_cstr +
> 1, ']');
> > -      if (!close_bracket_position) // if there is no ], this is a
> syntax error
> > +
> > +      size_t close_bracket_position = temp_expression.find(']', 1);
> > +      if (close_bracket_position ==
> > +          llvm::StringRef::npos) // if there is no ], this is a syntax
> error
> >       {
> > -        *first_unparsed = expression_cstr;
> >         *reason_to_stop =
> >             ValueObject::eExpressionPathScanEndReasonUnexpectedSymbol;
> >         *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
> > -        return ValueObjectSP();
> > +        return nullptr;
> >       }
> >
> > -      if (!separator_position || separator_position >
> close_bracket_position) {
> > +      llvm::StringRef bracket_expr =
> > +          temp_expression.slice(1, close_bracket_position);
> > +
> > +      // If this was an empty expression it would have been caught by
> the if
> > +      // above.
> > +      assert(!bracket_expr.empty());
> > +
> > +      if (!bracket_expr.contains('-')) {
> >         // if no separator, this is of the form [N].  Note that this
> cannot
> >         // be an unbounded range of the form [], because that case was
> handled
> >         // above with an unconditional return.
> > -        char *end = NULL;
> > -        unsigned long index = ::strtoul(expression_cstr + 1, &end, 0);
> > -        if (end != close_bracket_position) // if something weird is in
> > -                                           // our way return an error
> > -        {
> > -          *first_unparsed = expression_cstr;
> > +        unsigned long index = 0;
> > +        if (bracket_expr.getAsInteger(0, index)) {
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonUnexpectedSymbol;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > +          return nullptr;
> >         }
> > +
> >         // from here on we do have a valid index
> >         if (root_compiler_type_info.Test(eTypeIsArray)) {
> >           ValueObjectSP child_valobj_sp = root->GetChildAtIndex(index,
> true);
> > @@ -2549,15 +2540,15 @@ ValueObjectSP ValueObject::GetValueForEx
> >                   root->GetSyntheticValue()->GetChildAtIndex(index,
> true);
> >           if (child_valobj_sp) {
> >             root = child_valobj_sp;
> > -            *first_unparsed = end + 1; // skip ]
> > +            remainder =
> > +                temp_expression.substr(close_bracket_position + 1); //
> skip ]
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypePlain;
> >             continue;
> >           } else {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >                 ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           }
> >         } else if (root_compiler_type_info.Test(eTypeIsPointer)) {
> >           if (*what_next ==
> > @@ -2575,11 +2566,10 @@ ValueObjectSP ValueObject::GetValueForEx
> >             Error error;
> >             root = root->Dereference(error);
> >             if (error.Fail() || !root.get()) {
> > -              *first_unparsed = expression_cstr;
> >               *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
> >               *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -              return ValueObjectSP();
> > +              return nullptr;
> >             } else {
> >               *what_next = eExpressionPathAftermathNothing;
> >               continue;
> > @@ -2599,13 +2589,13 @@ ValueObjectSP ValueObject::GetValueForEx
> >             } else
> >               root = root->GetSyntheticArrayMember(index, true);
> >             if (!root.get()) {
> > -              *first_unparsed = expression_cstr;
> >               *reason_to_stop =
> >                   ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >               *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -              return ValueObjectSP();
> > +              return nullptr;
> >             } else {
> > -              *first_unparsed = end + 1; // skip ]
> > +              remainder =
> > +                  temp_expression.substr(close_bracket_position + 1);
> // skip ]
> >               *final_result =
> ValueObject::eExpressionPathEndResultTypePlain;
> >               continue;
> >             }
> > @@ -2613,15 +2603,13 @@ ValueObjectSP ValueObject::GetValueForEx
> >         } else if (root_compiler_type_info.Test(eTypeIsScalar)) {
> >           root = root->GetSyntheticBitFieldChild(index, index, true);
> >           if (!root.get()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >                 ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           } else // we do not know how to expand members of bitfields,
> so we
> >                  // just return and let the caller do any further
> processing
> >           {
> > -            *first_unparsed = end + 1; // skip ]
> >             *reason_to_stop = ValueObject::
> >                 eExpressionPathScanEndReasonBitfieldRangeOperatorMet;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeBitfield;
> > @@ -2630,13 +2618,13 @@ ValueObjectSP ValueObject::GetValueForEx
> >         } else if (root_compiler_type_info.Test(eTypeIsVector)) {
> >           root = root->GetChildAtIndex(index, true);
> >           if (!root.get()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >                 ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> >             return ValueObjectSP();
> >           } else {
> > -            *first_unparsed = end + 1; // skip ]
> > +            remainder =
> > +                temp_expression.substr(close_bracket_position + 1); //
> skip ]
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypePlain;
> >             continue;
> >           }
> > @@ -2649,80 +2637,64 @@ ValueObjectSP ValueObject::GetValueForEx
> >           if (root->HasSyntheticValue())
> >             root = root->GetSyntheticValue();
> >           else if (!root->IsSynthetic()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonSyntheticValueMissing;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           }
> >           // if we are here, then root itself is a synthetic VO.. should
> be good
> >           // to go
> >
> >           if (!root.get()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonSyntheticValueMissing;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           }
> >           root = root->GetChildAtIndex(index, true);
> >           if (!root.get()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >                 ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           } else {
> > -            *first_unparsed = end + 1; // skip ]
> > +            remainder =
> > +                temp_expression.substr(close_bracket_position + 1); //
> skip ]
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypePlain;
> >             continue;
> >           }
> >         } else {
> > -          *first_unparsed = expression_cstr;
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > -        }
> > -      } else // we have a low and a high index
> > -      {
> > -        char *end = NULL;
> > -        unsigned long index_lower = ::strtoul(expression_cstr + 1,
> &end, 0);
> > -        if (end != separator_position) // if something weird is in our
> > -                                       // way return an error
> > -        {
> > -          *first_unparsed = expression_cstr;
> > -          *reason_to_stop =
> > -              ValueObject::eExpressionPathScanEndReasonUnexpectedSymbol;
> > -          *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > +          return nullptr;
> >         }
> > -        unsigned long index_higher = ::strtoul(separator_position + 1,
> &end, 0);
> > -        if (end != close_bracket_position) // if something weird is in
> > -                                           // our way return an error
> > -        {
> > -          *first_unparsed = expression_cstr;
> > +      } else {
> > +        // we have a low and a high index
> > +        llvm::StringRef sleft, sright;
> > +        unsigned long low_index, high_index;
> > +        std::tie(sleft, sright) = bracket_expr.split('-');
> > +        if (sleft.getAsInteger(0, low_index) ||
> > +            sright.getAsInteger(0, high_index)) {
> >           *reason_to_stop =
> >               ValueObject::eExpressionPathScanEndReasonUnexpectedSymbol;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -          return ValueObjectSP();
> > +          return nullptr;
> >         }
> > -        if (index_lower > index_higher) // swap indices if required
> > -          std::swap(index_lower, index_higher);
> > +
> > +        if (low_index > high_index) // swap indices if required
> > +          std::swap(low_index, high_index);
> >
> >         if (root_compiler_type_info.Test(
> >                 eTypeIsScalar)) // expansion only works for scalars
> >         {
> > -          root =
> > -              root->GetSyntheticBitFieldChild(index_lower,
> index_higher, true);
> > +          root = root->GetSyntheticBitFieldChild(low_index, high_index,
> true);
> >           if (!root.get()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >                 ValueObject::eExpressionPathScanEndReasonNoSuchChild;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           } else {
> > -            *first_unparsed = end + 1; // skip ]
> >             *reason_to_stop = ValueObject::
> >                 eExpressionPathScanEndReasonBitfieldRangeOperatorMet;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeBitfield;
> > @@ -2739,17 +2711,15 @@ ValueObjectSP ValueObject::GetValueForEx
> >           Error error;
> >           root = root->Dereference(error);
> >           if (error.Fail() || !root.get()) {
> > -            *first_unparsed = expression_cstr;
> >             *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
> >             *final_result =
> ValueObject::eExpressionPathEndResultTypeInvalid;
> > -            return ValueObjectSP();
> > +            return nullptr;
> >           } else {
> >             *what_next = ValueObject::eExpressionPathAftermathNothing;
> >             continue;
> >           }
> >         } else {
> > -          *first_unparsed = expression_cstr;
> >           *reason_to_stop =
> >
>  ValueObject::eExpressionPathScanEndReasonArrayRangeOperatorMet;
> >           *final_result =
> ValueObject::eExpressionPathEndResultTypeBoundedRange;
> > @@ -2760,12 +2730,10 @@ ValueObjectSP ValueObject::GetValueForEx
> >     }
> >     default: // some non-separator is in the way
> >     {
> > -      *first_unparsed = expression_cstr;
> >       *reason_to_stop =
> >           ValueObject::eExpressionPathScanEndReasonUnexpectedSymbol;
> >       *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
> > -      return ValueObjectSP();
> > -      break;
> > +      return nullptr;
> >     }
> >     }
> >   }
> >
> > Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp?rev=287354&r1=287353&r2=287354&view=diff
> >
> ==============================================================================
> > --- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp (original)
> > +++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp Fri Nov 18
> 11:55:04 2016
> > @@ -280,7 +280,7 @@ bool lldb_private::formatters::LibCxxMap
> >   // die and free their memory
> >   m_pair_ptr = valobj_sp
> >                    ->GetValueForExpressionPath(
> > -                       ".__i_.__ptr_->__value_", nullptr, nullptr,
> nullptr,
> > +                       ".__i_.__ptr_->__value_", nullptr, nullptr,
> >                        ValueObject::GetValueForExpressionPathOptions()
> >                            .DontCheckDotVsArrowSyntax()
> >                            .SetSyntheticChildrenTraversal(
> > @@ -288,16 +288,18 @@ bool lldb_private::formatters::LibCxxMap
> >                                    SyntheticChildrenTraversal::None),
> >                        nullptr)
> >                    .get();
> > -
> > +
> >   if (!m_pair_ptr) {
> > -    m_pair_ptr = valobj_sp->GetValueForExpressionPath(".__i_.__ptr_",
> nullptr, nullptr, nullptr,
> > -
> ValueObject::GetValueForExpressionPathOptions()
> > -
> .DontCheckDotVsArrowSyntax()
> > -
> .SetSyntheticChildrenTraversal(
> > -
>              ValueObject::GetValueForExpressionPathOptions::
> > -
>              SyntheticChildrenTraversal::None),
> > -                                                      nullptr)
> > -    .get();
> > +    m_pair_ptr = valobj_sp
> > +                     ->GetValueForExpressionPath(
> > +                         ".__i_.__ptr_", nullptr, nullptr,
> > +                         ValueObject::GetValueForExpressionPathOptions()
> > +                             .DontCheckDotVsArrowSyntax()
> > +                             .SetSyntheticChildrenTraversal(
> > +
>  ValueObject::GetValueForExpressionPathOptions::
> > +                                     SyntheticChildrenTraversal::None),
> > +                         nullptr)
> > +                     .get();
> >     if (m_pair_ptr) {
> >       auto __i_(valobj_sp->GetChildMemberWithName(g___i_, true));
> >       lldb::TemplateArgumentKind kind;
> >
> > Modified: lldb/trunk/source/Symbol/Variable.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Variable.cpp?rev=287354&r1=287353&r2=287354&view=diff
> >
> ==============================================================================
> > --- lldb/trunk/source/Symbol/Variable.cpp (original)
> > +++ lldb/trunk/source/Symbol/Variable.cpp Fri Nov 18 11:55:04 2016
> > @@ -404,16 +404,14 @@ Error Variable::GetValuesForVariableExpr
> >                   const char *variable_sub_expr_path =
> >                       variable_expr_path + variable_name.size();
> >                   if (*variable_sub_expr_path) {
> > -                    const char *first_unparsed = nullptr;
> >                     ValueObject::ExpressionPathScanEndReason
> reason_to_stop;
> >                     ValueObject::ExpressionPathEndResultType
> final_value_type;
> >                     ValueObject::GetValueForExpressionPathOptions
> options;
> >                     ValueObject::ExpressionPathAftermath
> final_task_on_target;
> >
> >                     valobj_sp =
> variable_valobj_sp->GetValueForExpressionPath(
> > -                        variable_sub_expr_path, &first_unparsed,
> > -                        &reason_to_stop, &final_value_type, options,
> > -                        &final_task_on_target);
> > +                        variable_sub_expr_path, &reason_to_stop,
> > +                        &final_value_type, options,
> &final_task_on_target);
> >                     if (!valobj_sp) {
> >                       error.SetErrorStringWithFormat(
> >                           "invalid expression path '%s' for variable
> '%s'",
> >
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161118/bcea6653/attachment-0001.html>


More information about the lldb-commits mailing list