[Lldb-commits] [lldb] r287354 - Resubmit "Remove an output-parameter from Variable function".
Enrico Granata via lldb-commits
lldb-commits at lists.llvm.org
Fri Nov 18 13:16:55 PST 2016
I tend to agree with Jim's point. I have sometimes put in things that
look like "dead/vestigial code", which I am actually either planning to
use later on, or actively using somewhere else. This is a quite
realistic scenario due to Swift support for LLDB existing in an entirely
separate universe. I have multiple times committed changes to llvm.org
which are - on their face - glorified no-ops, but I actually end up
using on the other side of the Swift fence. Removing that kind of change
without asking first is A Bad Thing(TM).
Also, in the not-too-distant future, people who aren't the original
authors of this area of the code will end up having to own and make
calls about it. IMO, It becomes even more important to give everyone
advance notice at that point.
With all of that said, in this case, I believe the change itself is
fine. I have - admittedly - not delved deep into the mechanics of each
diff, but I am just making the "judgment call" on the removal of
first_unparsed. Removing that argument is fine.
As things stand, it is indeed unused. One day, we might decide we want
to do better diagnostics for summary formatters and/or frame variable
(e,g, be able to say things like "foo.bar->baz[3] not valid - reason:
baz doesn't have a member named [3] as it's not of array or pointer
type") - and then the combination of first_unparsed and reason_to_stop
would be exactly the data to use. With that said, we don't do this now,
and AFAIK there are no explicit plans around doing it.
tl;dr Thanks for asking, LGTM
On 11/18/16 12:32 PM, Zachary Turner wrote:
> 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 <mailto: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
> <mailto: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 <mailto: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 <mailto: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/d2a50fad/attachment-0001.html>
More information about the lldb-commits
mailing list