This is an archive of the discontinued LLVM Phabricator instance.

Fix bug in expression display when determining if a pointer should be treated as a string
ClosedPublic

Authored by paulmaybee on Jul 24 2015, 10:11 AM.

Details

Summary

Currently if the "first child" of the pointer is a char type then the pointer is displayed as a string. This test succeeds incorrectly when the pointer is to a structured type with a char type as its first field. Fix this by switching the test to retrieve the pointee type and checking that it is a char type.

Diff Detail

Event Timeline

paulmaybee retitled this revision from to Fix bug in expression display when determining if a pointer should be treated as a string.
paulmaybee updated this object.
paulmaybee added reviewers: abidh, ki.stfu, ChuckR.
paulmaybee added subscribers: lldb-commits, greggm.
ki.stfu edited edge metadata.Jul 24 2015, 11:45 AM

It looks strangely for me. If structure has the first element of type char then GetChildAtIndex(0) will refer to this element and GetType().GetBasicType() will be char. But if we have a pointer to that structure then I suppose GetChildAtIndex(0) will not be equal to the first element of structure. From my point of view, it should relate to this structure entirely, but not to its first element.

ki.stfu requested changes to this revision.Jul 24 2015, 11:46 AM
ki.stfu edited edge metadata.

Add add a test case for this please.

This revision now requires changes to proceed.Jul 24 2015, 11:46 AM
paulmaybee edited edge metadata.

Add test case

ki.stfu requested changes to this revision.Jul 27 2015, 12:04 AM
ki.stfu edited edge metadata.

See my inline comments.

test/tools/lldb-mi/variable/main.cpp
70–76

Use clang-format for formatting please.

83

a memory leak. just allocate it on the stack and then use "&nstr"

tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
139–141

Please wrap it into IsPointeeCharType():

if (m_bHandleCharType && IsPointeeCharType())
364

Change to "Static":

// Type:    Static.
404

Add IsPointeeCharType() function after IsFirstChildCharType():

//++ ------------------------------------------------------------------------------------
// Details: Retrieve the flag stating whether pointee object of *this object is a char
//          type or some other type. Returns false if there are not children. Char type
//          can be signed or unsigned.
// Type:    Method.
// Args:    None.          
// Return:  bool    - True = Yes is a char type, false = some other type.
// Throws:  None.
//--
bool
CMICmnLLDBUtilSBValue::IsPointeeCharType(void) const
{
    const MIuint nChildren = m_rValue.GetNumChildren();

    // Is it a basic type
    if (nChildren == 0)
        return false;

    const lldb::BasicType eType = m_rValue.GetType().GetPointeeType().GetBasicType();
    return IsCharBasicType(eType);
}
tools/lldb-mi/MICmnLLDBUtilSBValue.h
57

Add

bool IsPointeeCharType(void) const;

after these lines:

CMIUtilString GetTypeName(void) const;
CMIUtilString GetTypeNameDisplay(void) const;
bool IsCharType(void) const;
bool IsFirstChildCharType(void) const;
60

Move to separate section:

  // Statics:
private:
  static bool IsCharBasicType(lldb::BasicType eType);
This revision now requires changes to proceed.Jul 27 2015, 12:04 AM
paulmaybee edited edge metadata.

Add IsPointeeCharType

ki.stfu accepted this revision.Jul 27 2015, 10:14 PM
ki.stfu edited edge metadata.

Add a section title and go ahead.

tools/lldb-mi/MICmnLLDBUtilSBValue.h
61

Forgot a section title:

  // Statics:
private:
  static bool IsCharBasicType(lldb::BasicType eType);
This revision is now accepted and ready to land.Jul 27 2015, 10:14 PM
paulmaybee updated this revision to Diff 30830.Jul 28 2015, 9:44 AM
paulmaybee edited edge metadata.

Added the section title.
If you are ok with this now then can you please check it in?

Thanks.

ki.stfu closed this revision.Jul 29 2015, 10:33 PM