[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added inline comments.


================
Comment at: lldb/source/Target/Target.cpp:4520
   const uint32_t idx = ePropertyMaxSummaryLength;
-  return m_collection_sp->GetPropertyAtIndexAsSInt64(idx).value_or(
-      g_target_properties[idx].default_uint_value);
+  return GetPropertyAtIndexAs<uint64_t>(
+      idx, g_target_properties[idx].default_uint_value);
----------------
I think this is causing `settings set target.max-string-summary-length` not to work properly.

As far as I can tell, the problem is that `GetPropertyAtIndexAs<uint64_t>` will end up calling `OptionValue::GetAsUInt64()` which checks the declared type of the property and returns null if there's a mismatch. `target.max-string-summary-length` is, for some reason, defined as signed, so this will always return the default value regardless of what it was set to.

You should be able to reproduce it by writing a simple program with a `std::string`, setting `target.max-string-summary-length` to some small value and checking that the string is not truncated.

`GetMaximumMemReadSize`, just below this one, seems to be mismatched too because the diff shows a change from `GetPropertyAtIndexAsSInt64` to `GetPropertyAtIndexAs<uint64_t>` but I haven't actually tested that one. It would probably be a good idea to double check the rest of the patch looking for other mismatches.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149774/new/

https://reviews.llvm.org/D149774