Skip to content

[cxx-interop] Update CxxMethodBridging with snake_case #41617

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

Closed
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
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsFrontend.def
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ ERROR(batch_scan_input_file_corrupted,none,
(StringRef))

ERROR(unknown_platform_name, none,
"unkown platform name %0", (StringRef))
"unknown platform name %0", (StringRef))

ERROR(unknown_swift_module_name, none,
"cannot find Swift module with name %0", (StringRef))
Expand Down
74 changes: 57 additions & 17 deletions include/swift/ClangImporter/CXXMethodBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,70 +8,92 @@
#include <string>
namespace swift {
struct CXXMethodBridging {
enum class Kind { unkown, getter, setter, subscript };
enum class Kind { unknown, getter, setter, subscript };

enum class NameKind { unkown, snake, lower, camel, title };
enum class NameKind { unknown, snake, lower, camel, title };

CXXMethodBridging(const clang::CXXMethodDecl *method) : method(method) {}

Kind classify() {
if (nameIsBlacklist())
return Kind::unkown;
return Kind::unknown;

// this should be handled as snake case. See: rdar://89453010
// case. In the future we could
// import these too, though.
auto nameKind = classifyNameKind();
if (nameKind != NameKind::title && nameKind != NameKind::camel &&
nameKind != NameKind::lower)
return Kind::unkown;
nameKind != NameKind::lower && nameKind != NameKind::snake)
return Kind::unknown;

if (getClangName().startswith_insensitive("set")) {
// Setters only have one parameter.
if (method->getNumParams() != 1)
return Kind::unkown;
return Kind::unknown;

// rdar://89453106 (We need to handle imported properties that return a
// reference)
if (method->getParamDecl(0)->getType()->isReferenceType())
return Kind::unkown;
return Kind::unknown;

return Kind::setter;
}

// Getters and subscripts cannot return void.
if (method->getReturnType()->isVoidType())
return Kind::unkown;
return Kind::unknown;

if (getClangName().startswith_insensitive("get")) {
// Getters cannot take arguments.
if (method->getNumParams() != 0)
return Kind::unkown;
return Kind::unknown;

// rdar://89453106 (We need to handle imported properties that return a
// reference)
if (method->getReturnType()->isReferenceType())
return Kind::unkown;
return Kind::unknown;

return Kind::getter;
}

// rdar://89453187 (Add subscripts clarification to CXXMethod Bridging to
// clean up importDecl)
return Kind::unkown;
return Kind::unknown;
}

NameKind classifyNameKind() {
bool allLower = llvm::all_of(getClangName(), islower);
bool hasUpper = false;

if (getClangName().empty())
return NameKind::unkown;
for (std::size_t i = 0; i < getClangName().size(); i++) {
if (std::isupper(getClangName()[i])) {
hasUpper = true;
}
}

if (getClangName().contains('_'))
return allLower ? NameKind::snake : NameKind::unkown;
// if the string is empty
if (getClangName().empty()) {
return NameKind::unknown;
}

if (allLower)
return NameKind::lower;
// if one of the elements is an underscore
if (getClangName().contains('_')) {
if (allLower) {
return NameKind::snake;
} else {
// if there is an underscore and contains an uppercase
if (hasUpper) {
return NameKind::unknown;
} else {
// if there is an underscore, there are no uppercase letters and can
// contain numbers
return NameKind::snake;
}

// final check
return allLower ? NameKind::snake : NameKind::unknown;
}
}

return islower(getClangName().front()) ? NameKind::camel : NameKind::title;
}
Expand Down Expand Up @@ -103,6 +125,24 @@ struct CXXMethodBridging {
// The first character is always lowercase.
output.front() = std::tolower(output.front());

if (classifyNameKind() == NameKind::snake) {
for (std::size_t i = 0; i < output.size(); i++) {
size_t next = i + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the last character is an underscore, output[next] isn't checked to exist and is erased. We should check the validity of next.

Suggested change
size_t next = i + 1;
size_t next = i + 1;
if (output[next] == output.end())
return output;

if (output[i] == '_') {
// if the first element is an underscore, remove it
if (i == 0) {
output.erase(i, 1);
} else {
// if the current element is an underscore, capitalize the element
// next to it, and remove the extra element
output[i] = std::toupper(output[next]);
output.erase(next, 1);
}
}
}
return output;
}

// We already lowercased the first element, so start at one. Look at the
// current element and the next one. To handle cases like UTF8String, start
// making all the uppercase characters lower, until we see an upper case
Expand Down
2 changes: 1 addition & 1 deletion test/Index/index_generic_params.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ extension Wrapper2.NonGenericWrapped where Wrapper2Param: P1 {
func bar(x: Wrapper2Param.Assoc) {}
}

// MARK: - Test extending an unkown type
// MARK: - Test extending an unknown type

// Check that we don't crash. We don't expect the generic params to show up in the index.
extension MyUnknownType where Wrapper2Param: P1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ struct LongNameAllUpper {
};

struct UpperCaseMix {
mutable int value = 42;
int getFoo() const { return value; }
void SetFoo(int v) { value = v; }
mutable int value = 42;
int getFoo() const { return value; }
void SetFoo(int v) { value = v; }
};

struct UpperCaseGetterSetter {
mutable int value = 42;
int GetFoo() const { return value; }
void SetFoo(int v) { value = v; }
mutable int value = 42;
int GetFoo() const { return value; }
void SetFoo(int v) { value = v; }
};

struct GetterOnly {
Expand Down Expand Up @@ -191,4 +191,16 @@ class PrivatePropertyWithSameName {
void setValue(int i);
};

struct SnakeCaseGetterSetter {
mutable int value;
int get_foo() const { return value; }
void set_foo(int v) { value = v; }
};

struct SnakeCaseUTF8Str {
mutable int value;
int get_utf8_string() const { return value; }
void set_utf8_string(int v) { value = v; }
};

#endif // SWIFT_IMPLICIT_COMPUTED_PROPERTIES_H
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
// CHECK: struct IntGetterSetterSnakeCase {
// CHECK-NEXT: init()
// CHECK-NEXT: init(val: Int32)
// CHECK-NEXT: var x: Int32
// CHECK-NEXT: func get_x() -> Int32
// CHECK-NEXT: mutating func set_x(_ v: Int32)
// CHECK-NEXT: var val: Int32
Expand Down Expand Up @@ -265,3 +266,21 @@
// CHECK-NEXT: func getValue() -> Int32
// CHECK-NEXT: mutating func setValue(_ i: Int32)
// CHECK-NEXT: }

// CHECK: struct SnakeCaseGetterSetter {
// CHECK-NEXT: init()
// CHECK-NEXT: init(value: Int32)
// CHECK-NEXT: var foo: Int32
// CHECK-NEXT: func get_foo() -> Int32
// CHECK-NEXT: mutating func set_foo(_ v: Int32)
// CHECK-NEXT: var value: Int32
// CHECK-NEXT: }

// CHECK: struct SnakeCaseUTF8Str {
// CHECK-NEXT: init()
// CHECK-NEXT: init(value: Int32)
// CHECK-NEXT: utf8String: Int32
// CHECK-NEXT: func get_utf8_string() -> Int32
// CHECK-NEXT: mutating func set_utf8_string(_ v: Int32)
// CHECK-NEXT: var value: Int32
// CHECK-NEXT: }
14 changes: 14 additions & 0 deletions test/Interop/Cxx/ergonomics/implicit-computed-properties.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,18 @@ ImplicitComputedPropertiesTestSuite.test("non trivial") {
expectEqual(Object.x.value, 20)
}

ImplicitComputedPropertiesTestSuite.test("SnakeCaseGetterSetter") {
var object = SnakeCaseGetterSetter()
expectEqual(object.foo, 42)
object.foo = 32
expectEqual(object.foo, 32)
}

ImplicitComputedPropertiesTestSuite.test("SnakeCaseUTF8Str") {
var object = SnakeCaseUTF8Str()
expectEqual(object.utf8string, 42)
object.utf8string = 32
expectEqual(object.utf8string, 32)
}

runAllTests()
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ bb0(%arg0 : @owned $MyKlass, %arg1 : @owned $MyKlass, %arg2 : @owned $MyKlass):
return %52 : $()
}

sil [ossa] @unkown_use : $@convention(thin) (@owned MyKlass) -> () {
sil [ossa] @unknown_use : $@convention(thin) (@owned MyKlass) -> () {
bb0(%arg0 : @owned $MyKlass):
%0 = function_ref @swift_bufferAllocate : $@convention(thin) () -> @owned _ContiguousArrayStorage<MyKlass>
%1 = integer_literal $Builtin.Word, 1
Expand Down
4 changes: 2 additions & 2 deletions test/Sema/availability_define.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ public func onMyProjectV3() {}
// expected-error @-1 {{expected version number}}
public func brokenVersion() {}

@available(_unkownMacro, *) // expected-error {{expected declaration}}
@available(_unknownMacro, *) // expected-error {{expected declaration}}
// expected-error @-1 {{expected 'available' option such as 'unavailable', 'introduced', 'deprecated', 'obsoleted', 'message', or 'renamed'}}
public func unkownMacro() {}
public func unknownMacro() {}

@available(_iOS9) // expected-error {{must handle potential future platforms with '*'}}
public func noOtherOSes() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ def test(self):
[option.option_string])
# The argument should never show up in the namespace
self.assertFalse(hasattr(namespace, option.dest))
# It should instead be forwareded to unkown_args
# It should instead be forwareded to unknown_args
self.assertEqual(unknown_args, [option.option_string])

return test
Expand Down