This is an archive of the discontinued LLVM Phabricator instance.

[lldb][NFC] Remove stride parameter from GetArrayElementType
ClosedPublic

Authored by teemperor on Jul 22 2020, 2:38 AM.

Details

Summary

This parameter isn't used anywhere in LLDB nor the Swift downstream branch.
It also doesn't really fit into the TypeSystem APIs that usually don't return additional
related functionality via some output parameters. Also the implementations already
states that the calculated value there is wrong.

Let's remove it. If we need this functionality at some point then Swift's much nicer
GetByteStride function seems like the way to go.

Diff Detail

Event Timeline

teemperor created this revision.Jul 22 2020, 2:38 AM

(I assume this will go in after D84267, so I'll revert the added ExecutionContext parameter to that function too).

aprantl accepted this revision.Jul 22 2020, 8:34 AM

I didn't realize that is was entirely unused.

This revision is now accepted and ready to land.Jul 22 2020, 8:34 AM

Did you do some archeology to figure out why this was added? I don't want to pointlessly break some out-of-tree plugin.

Did you do some archeology to figure out why this was added? I don't want to pointlessly break some out-of-tree plugin.

It was added when the function was initially added here:

commit 9128ee2f7accbb6225858416c8a956e6102b86b8
Author: Enrico Granata <granata.enrico@gmail.com>
Date:   Tue Sep 6 19:20:51 2011 +0000

    Redesign of the interaction between Python and frozen objects:
     - introduced two new classes ValueObjectConstResultChild and ValueObjectConstResultImpl: the first one is a ValueObjectChild obtained from
       a ValueObjectConstResult, the second is a common implementation backend for VOCR and VOCRCh of method calls meant to read through pointers stored
       in frozen objects ; now such reads transparently move from host to target as required
     - as a consequence of the above, removed code that made target-memory copies of expression results in several places throughout LLDB, and also
       removed code that enabled to recognize an expression result VO as such
     - introduced a new GetPointeeData() method in ValueObject that lets you read a given amount of objects of type T from a VO
       representing a T* or T[], and doing dereferences transparently
       in private layer it returns a DataExtractor ; in public layer it returns an instance of a newly created lldb::SBData
     - as GetPointeeData() does the right thing for both frozen and non-frozen ValueObject's, reimplemented ReadPointedString() to use it
       en lieu of doing the raw read itself
     - introduced a new GetData() method in ValueObject that lets you get a copy of the data that backs the ValueObject (for pointers,
       this returns the address without any previous dereferencing steps ; for arrays it actually reads the whole chunk of memory)
       in public layer this returns an SBData, just like GetPointeeData()
     - introduced a new CreateValueFromData() method in SBValue that lets you create a new SBValue from a chunk of data wrapped in an SBData
       the limitation to remember for this kind of SBValue is that they have no address: extracting the address-of for these objects (with any
       of GetAddress(), GetLoadAddress() and AddressOf()) will return invalid values
     - added several tests to check that "p"-ing objects (STL classes, char* and char[]) will do the right thing
    Solved a bug where global pointers to global variables were not dereferenced correctly for display
    New target setting "max-string-summary-length" gives the maximum number of characters to show in a string when summarizing it, instead of the hardcoded 128
    Solved a bug where the summary for char[] and char* would not be shown if the ValueObject's were dumped via the "p" command
    Removed m_pointers_point_to_load_addrs from ValueObject. Introduced a new m_address_type_of_children, which each ValueObject can set to tell the address type
     of any pointers and/or references it creates. In the current codebase, this is load address most of the time (the only notable exception being file
     addresses that generate file address children UNLESS we have a live process)
    Updated help text for summary-string
    Fixed an issue in STL formatters where std::stlcontainer::iterator would match the container's synthetic children providers
    Edited the syntax and help for some commands to have proper argument types

The parameter was also already unused when it was added (alongside the comment that the calculated value was wrong). Technically it was 'used' as it was a reference back then, but the patch just passes some variable in that was then unused.

Thanks! I was mostly worried that it was introduced for Rust/Delphi/... support, but that does not seem to be the case.

Herald added a project: Restricted Project. · View Herald TranscriptAug 17 2020, 1:20 AM