Skip to content

[lldb] Calculate stride of clang types on emplaceClangTypeInfo. #3914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -664,17 +664,22 @@ SwiftLanguageRuntimeImpl::emplaceClangTypeInfo(
m_clang_type_info.insert({clang_type.GetOpaqueQualType(), llvm::None});
return nullptr;
}
assert(*bit_align % 8 == 0 && "Bit alignment no a multiple of 8!");
auto byte_align = *bit_align / 8;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is bit_align guaranteed to be a multiple of 8?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I just moved that from a block of code below:

         swift::reflection::TypeInfo(swift::reflection::TypeInfoKind::Builtin,
                                      *byte_size, *bit_align / 8, 0, 0, true)});

I'll find out.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we have an alignment that's not expressible in bytes though?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't think so. Sounds like this calls for an assert.

// The stride is the size rounded up to alignment.
size_t byte_stride = llvm::alignTo(*byte_size, byte_align);
if (fields.empty()) {
auto it_b = m_clang_type_info.insert(
{clang_type.GetOpaqueQualType(),
swift::reflection::TypeInfo(swift::reflection::TypeInfoKind::Builtin,
*byte_size, *bit_align / 8, 0, 0, true)});
*byte_size, byte_align, byte_stride, 0,
true)});
return &*it_b.first->second;
}
auto it_b = m_clang_record_type_info.insert(
{clang_type.GetOpaqueQualType(),
swift::reflection::RecordTypeInfo(
*byte_size, *bit_align / 8, 0, 0, false,
*byte_size, byte_align, byte_stride, 0, false,
swift::reflection::RecordKind::Struct, fields)});
return &*it_b.first->second;
}
Expand Down
4 changes: 4 additions & 0 deletions lldb/test/API/lang/swift/array_bridged_enum/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
SWIFT_SOURCES := main.swift
SWIFT_BRIDGING_HEADER := bridging-header.h
SWIFT_OBJC_INTEROP := 1
include Makefile.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import lldbsuite.test.lldbinline as lldbinline
from lldbsuite.test.decorators import *

lldbinline.MakeInlineTest(__file__, globals(), decorators=[swiftTest, skipUnlessFoundation])
12 changes: 12 additions & 0 deletions lldb/test/API/lang/swift/array_bridged_enum/bridging-header.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#ifndef bridging_header_h
#define bridging_header_h
Comment on lines +1 to +2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do bridging headers need these? Normal objc doesn't need these because #import has this behavior.


#import <Foundation/Foundation.h>

typedef NS_CLOSED_ENUM(NSInteger, SampleEnum) {
SampleEnumZero,
SampleEnumOne,
SampleEnumTwo,
SampleEnumThree,
};
#endif /* bridging_header_h */
5 changes: 5 additions & 0 deletions lldb/test/API/lang/swift/array_bridged_enum/main.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
let array = [SampleEnum.zero, SampleEnum.one, SampleEnum.two, SampleEnum.three]
print(array) //%self.expect('frame variable array[0]', substrs=['[0]', '.SampleEnumZero'])
//%self.expect('frame variable array[1]', substrs=['[1]', '.SampleEnumOne'])
//%self.expect('frame variable array[2]', substrs=['[2]', '.SampleEnumTwo'])
//FIXME: frame variable array[3] returns [.SampleEnumOne, .SampleEnumTwo]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you find out why this is happening, I'd love to know what the reason is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the FIXME? I saw it when I wrote the test, I have no idea why it's happening yet.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the FIXME, I'd like to know what the underlying issue is

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file a radar for this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do