Skip to content
New issue

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

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

Already on GitHub? # to your account

Rule Request: Prefer Self over type(of: self) #3003

Closed
2 tasks done
marcelofabri opened this issue Jan 2, 2020 · 5 comments · Fixed by #3006
Closed
2 tasks done

Rule Request: Prefer Self over type(of: self) #3003

marcelofabri opened this issue Jan 2, 2020 · 5 comments · Fixed by #3006
Labels
rule-request Requests for a new rules.

Comments

@marcelofabri
Copy link
Collaborator

New Issue Checklist

#2944 and #2921 are related, but I think this should be a separate rule.

New rule request

  1. Why should this rule be added? Share links to existing discussion about what
    the community thinks about this.

Swift 5.1 introduces the Self type, which can be used in place of type(of: self).

  1. Provide several examples of what would and wouldn't trigger violations.
// ok
class A {
  func foo() {
    Self.bar() 
   }
}

class A {
  func foo(param: B) {
    type(of: param).bar() 
   }
}

// triggers
class A {
  func foo() {
    type(of: self).bar() 
   }
}
  1. Should the rule be configurable, if so what parameters should be configurable?

Just severity.

  1. Should the rule be opt-in or enabled by default? Why?
    See README.md for guidelines on when to mark a rule as opt-in.

Opt-in.

@jpsim
Copy link
Collaborator

jpsim commented Jan 3, 2020

Why opt-in? Are there cases where this isn't preferred or where the semantics are different?

@jpsim
Copy link
Collaborator

jpsim commented Jan 3, 2020

Why opt-in?

Ah I know now, it's for support for Swift < 5.1

@marcelofabri
Copy link
Collaborator Author

Also I’m not sure if the community has settled on one or another

@cltnschlosser
Copy link
Contributor

cltnschlosser commented Jan 4, 2020

Is there a performance benefit to Self? or does the runtime still need to look up the type.

EDIT: Spent the last few minutes playing around with some test cases, and looking at optimized sil. Looks like the compiler is smart enough to optimize away the type(of:) when it can. Also interestingly there is different sil for Self vs type(of:). Not sure what the difference is.

Test File:

final class ControlClass {
	func test() {
		ControlClass.bar()
	}

	static func bar() {}
}

final class TypeOfClass {
	func test() {
		type(of: self).bar()
	}

	static func bar() {}
}

final class SelfClass {
	func test() {
		Self.bar()
	}

	static func bar() {}
}

class BaseClass {
	func test() {
		Self.bar()
	}

	class func bar() {}
}

class SubClass: BaseClass {
	override class func bar() {}
}

class TBaseClass {
	func test() {
		type(of: self).bar()
	}

	class func bar() {}
}

class TSubClass: TBaseClass {
	override class func bar() {}
}

ControlClass().test()
TypeOfClass().test()
SelfClass().test()
SubClass().test()
TSubClass().test()

Emit sil command: swiftc -O -emit-sil /tmp/test.swift

The interesting cases:

Self:

// BaseClass.test()
sil hidden @$s4test9BaseClassCAAyyF : $@convention(method) (@guaranteed BaseClass) -> () {
// %0                                             // users: %2, %1
bb0(%0 : $BaseClass):
  debug_value %0 : $BaseClass, let, name "self", argno 1 // id: %1
  %2 = metatype $@thick @dynamic_self BaseClass.Type // type-defs: %0; user: %3
  %3 = upcast %2 : $@thick @dynamic_self BaseClass.Type to $@thick BaseClass.Type // users: %10, %4, %9
  checked_cast_br [exact] %3 : $@thick BaseClass.Type to $@thick SubClass.Type, bb2, bb3 // id: %4

bb1:                                              // Preds: bb2 bb3
  %5 = tuple ()                                   // user: %6
  return %5 : $()                                 // id: %6

bb2(%7 : $@thick SubClass.Type):                  // Preds: bb0
  br bb1                                          // id: %8

bb3:                                              // Preds: bb0
  %9 = class_method %3 : $@thick BaseClass.Type, #BaseClass.bar!1 : (BaseClass.Type) -> () -> (), $@convention(method) (@thick BaseClass.Type) -> () // user: %10
  %10 = apply %9(%3) : $@convention(method) (@thick BaseClass.Type) -> ()
  br bb1                                          // id: %11
} // end sil function '$s4test9BaseClassCAAyyF'

type(of: self)

// TBaseClass.test()
sil hidden @$s4test10TBaseClassCAAyyF : $@convention(method) (@guaranteed TBaseClass) -> () {
// %0                                             // users: %2, %1
bb0(%0 : $TBaseClass):
  debug_value %0 : $TBaseClass, let, name "self", argno 1 // id: %1
  %2 = value_metatype $@thick TBaseClass.Type, %0 : $TBaseClass // users: %12, %8, %3, %11
  checked_cast_br [exact] %2 : $@thick TBaseClass.Type to $@thick TBaseClass.Type, bb2, bb3 // id: %3

bb1:                                              // Preds: bb2 bb4 bb5
  %4 = tuple ()                                   // user: %5
  return %4 : $()                                 // id: %5

bb2(%6 : $@thick TBaseClass.Type):                // Preds: bb0
  br bb1                                          // id: %7

bb3:                                              // Preds: bb0
  checked_cast_br [exact] %2 : $@thick TBaseClass.Type to $@thick TSubClass.Type, bb4, bb5 // id: %8

bb4(%9 : $@thick TSubClass.Type):                 // Preds: bb3
  br bb1                                          // id: %10

bb5:                                              // Preds: bb3
  %11 = class_method %2 : $@thick TBaseClass.Type, #TBaseClass.bar!1 : (TBaseClass.Type) -> () -> (), $@convention(method) (@thick TBaseClass.Type) -> () // user: %12
  %12 = apply %11(%2) : $@convention(method) (@thick TBaseClass.Type) -> ()
  br bb1                                          // id: %13
} // end sil function '$s4test10TBaseClassCAAyyF'

For the most part it's the same, I see 2 differences.

  1. Difference in getting the type
    Self uses:
%2 = metatype $@thick @dynamic_self BaseClass.Type // type-defs: %0; user: %3
%3 = upcast %2 : $@thick @dynamic_self BaseClass.Type to $@thick BaseClass.Type // users: %10, %4, %9

type(of: self) uses:

%2 = value_metatype $@thick TBaseClass.Type, %0 : $TBaseClass // users: %12, %8, %3, %11
  1. type(of: self) checks for both TBaseClass and TSubClass before falling back to what appears to be the slow path. Whereas Self checks only SubClass, so BaseClass would fall into the slow path always.

Note: sil probably doesn't tell the whole story.

TLDR: It's basically the same. Trust the compiler to make the right optimizations. (It is interesting that they are optimized differently. If one is better than the other, then this may be an area for improvement).

@jpsim
Copy link
Collaborator

jpsim commented Jan 4, 2020

Thanks for the thoughtful analysis, @cltnschlosser ! ❤️

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants