[Lldb-commits] [lldb] r351250 - Simplify Value::GetValueByteSize()

[Adrian Prantl via lldb-commits]

> On Jan 16, 2019, at 4:22 AM, Pavel Labath <pavel at labath.sk> wrote:
> 
> Hi Adrian, Zachary,
> 
> this commit breaks the SymbolFile/NativePDB/function-types-builtins.cpp test <http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/630>. This is due do a slight behavioral change where previously this function would consider a value 0 from GetByteSize to be failure, but now it's considered a success. This seemed to matter for function references, where it changed the output when displaying them (though I can't really say whether it's for the better or worse).
> 
> I am not sure whether this change was intentional or not (it would seem to be in line with the overall direction you're going in with the Optional patches, but the commit message just says "simplify"), and I am also not sure whether the that test intentionally meant to check this error message (it seems somewhat irrelevant to that test, but maybe Zachary put it there for some reason). For the time being, I've reverted the patch to keep the build green.
> 
> BTW: You should be able to reproduce this failure if you add lld to your build config. Alternatively you can just compile that source file for darwin, as the same behavioral change is present with dwarf debug info too.
> 

On Darwin I'm getting an also very misleading

(lldb) target var ref
(void (&)(bool)) ref = 0x0000000100000920 (&::ref = <out of memory>)

with and without the patch.

Which error message are you getting on Windows with my patch applied?

I'll see if I can get LLDB to produce a more meaningful error message on Darwin, perhaps this will also improve the error on Windows.

-- adrian

> 
> On 15/01/2019 22:26, Adrian Prantl via lldb-commits wrote:
>> Author: adrian
>> Date: Tue Jan 15 13:26:03 2019
>> New Revision: 351250
>> URL: http://llvm.org/viewvc/llvm-project?rev=351250&view=rev
>> Log:
>> Simplify Value::GetValueByteSize()
>> Modified:
>>     lldb/trunk/source/Core/Value.cpp
>> Modified: lldb/trunk/source/Core/Value.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Value.cpp?rev=351250&r1=351249&r2=351250&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Core/Value.cpp (original)
>> +++ lldb/trunk/source/Core/Value.cpp Tue Jan 15 13:26:03 2019
>> @@ -210,35 +210,31 @@ bool Value::ValueOf(ExecutionContext *ex
>>  }
>>    uint64_t Value::GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx) {
>> -  uint64_t byte_size = 0;
>> -
>>    switch (m_context_type) {
>>    case eContextTypeRegisterInfo: // RegisterInfo *
>> -    if (GetRegisterInfo())
>> -      byte_size = GetRegisterInfo()->byte_size;
>> +    if (GetRegisterInfo()) {
>> +      if (error_ptr)
>> +        error_ptr->Clear();
>> +      return GetRegisterInfo()->byte_size;
>> +    }
>>      break;
>>      case eContextTypeInvalid:
>>    case eContextTypeLLDBType: // Type *
>>    case eContextTypeVariable: // Variable *
>>    {
>> -    const CompilerType &ast_type = GetCompilerType();
>> -    if (ast_type.IsValid())
>> -      if (llvm::Optional<uint64_t> size = ast_type.GetByteSize(
>> -              exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
>> -        byte_size = *size;
>> -  } break;
>> -  }
>> -
>> -  if (error_ptr) {
>> -    if (byte_size == 0) {
>> -      if (error_ptr->Success())
>> -        error_ptr->SetErrorString("Unable to determine byte size.");
>> -    } else {
>> -      error_ptr->Clear();
>> +    auto *scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
>> +    if (llvm::Optional<uint64_t> size = GetCompilerType().GetByteSize(scope)) {
>> +      if (error_ptr)
>> +        error_ptr->Clear();
>> +      return *size;
>>      }
>> +    break;
>> +  }
>>    }
>> -  return byte_size;
>> +  if (error_ptr && error_ptr->Success())
>> +    error_ptr->SetErrorString("Unable to determine byte size.");
>> +  return 0;
>>  }
>>    const CompilerType &Value::GetCompilerType() {
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>