Custom summary for Mirrors?

Austin · 2015-12-30T06:38:02.000Z

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to the
standard library's Mirror type a 'summary' property, and the option to
initialize a Mirror with a custom summary. If no custom summary is
provided, the summary would default to the string produced by calling
String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard
library: CustomReflectable, which is publicly exposed and relies on the
conforming type creating a Mirror object, and _Reflectable, which relies on
the conforming type having a companion type conforming to _MirrorType. A
short-term goal is to migrate the standard library's types off the
_Reflectable API and have them use the CustomReflectable API, and changing
dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called
"summary". (This is where e.g. "4 elements" comes from when you dump() an
array.) "summary" is absent from Mirror or any types related to
CustomReflectable. I asked Joe Groff about this and the rationale was that
it was deemed too similar to debugDescription (or String(reflecting: foo))
to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often print
out a description of their children as part of their debugDescription or
description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin

Joe_Groff · 2015-12-30T17:55:22.000Z

I believe 'summary' is obsolete, and you're supposed to use Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

···

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution <swift-evolution@swift.org> wrote:

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to the standard library's Mirror type a 'summary' property, and the option to initialize a Mirror with a custom summary. If no custom summary is provided, the summary would default to the string produced by calling String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard library: CustomReflectable, which is publicly exposed and relies on the conforming type creating a Mirror object, and _Reflectable, which relies on the conforming type having a companion type conforming to _MirrorType. A short-term goal is to migrate the standard library's types off the _Reflectable API and have them use the CustomReflectable API, and changing dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called "summary". (This is where e.g. "4 elements" comes from when you dump() an array.) "summary" is absent from Mirror or any types related to CustomReflectable. I asked Joe Groff about this and the rationale was that it was deemed too similar to debugDescription (or String(reflecting: foo)) to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often print out a description of their children as part of their debugDescription or description, redundant when using an API like dump() which provides a structural representation of the children of the subject. In such cases a lighter-weight description (like "3 elements") might be more appropriate to represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible, CustomDebugStringConvertible, Streamable, etc. Having a custom summary for these types customized by the corresponding Mirror would allow for a 'pretty' representation during reflection in lieu of the ugly one generated by the runtime without making more substantial changes to the API which might break third-party code (such as conforming CGRect to any of the aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for major design changes, so this would be a minor transient change to make the API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is meaningful and worthwhile, or if you have any questions.

Best,
Austin
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

jrose · 2016-01-05T23:22:37.000Z

Getting custom summaries for the common CG types certainly seems reasonable. We'd have to get approval from the appropriate teams at Apple, but I can't see any objections.

Jordan

···

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

I believe 'summary' is obsolete, and you're supposed to use Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to the standard library's Mirror type a 'summary' property, and the option to initialize a Mirror with a custom summary. If no custom summary is provided, the summary would default to the string produced by calling String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard library: CustomReflectable, which is publicly exposed and relies on the conforming type creating a Mirror object, and _Reflectable, which relies on the conforming type having a companion type conforming to _MirrorType. A short-term goal is to migrate the standard library's types off the _Reflectable API and have them use the CustomReflectable API, and changing dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called "summary". (This is where e.g. "4 elements" comes from when you dump() an array.) "summary" is absent from Mirror or any types related to CustomReflectable. I asked Joe Groff about this and the rationale was that it was deemed too similar to debugDescription (or String(reflecting: foo)) to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often print out a description of their children as part of their debugDescription or description, redundant when using an API like dump() which provides a structural representation of the children of the subject. In such cases a lighter-weight description (like "3 elements") might be more appropriate to represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible, CustomDebugStringConvertible, Streamable, etc. Having a custom summary for these types customized by the corresponding Mirror would allow for a 'pretty' representation during reflection in lieu of the ugly one generated by the runtime without making more substantial changes to the API which might break third-party code (such as conforming CGRect to any of the aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for major design changes, so this would be a minor transient change to make the API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is meaningful and worthwhile, or if you have any questions.

Best,
Austin
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

Austin · 2016-01-06T01:28:15.000Z

Hi Joe,

I respect the choice of the team to use Custom[Debug]StringConvertible in
lieu of summary. At the same time, in my opinion the output of dump() has
become significantly more difficult to read (c.f. unit tests in
Migrate mirrors to use CustomReflectable API, rewrite dump() by austinzheng · Pull Request #838 · apple/swift · GitHub). Would you and the team be
open to exploring alternative solutions that improve the readability of
dump() without increasing API surface area? For example, perhaps the
reflection machinery itself should have special handling for some of the
built-in types. If not, I'll consider this discussion thread complete.

Thanks,
Austin

···

On Tue, Jan 5, 2016 at 3:22 PM, Jordan Rose <jordan_rose@apple.com> wrote:

Getting custom summaries for the common CG types certainly seems
reasonable. We'd have to get approval from the appropriate teams at Apple,
but I can't see any objections.

Jordan

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution < > swift-evolution@swift.org> wrote:

I believe 'summary' is obsolete, and you're supposed to use
Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution < > swift-evolution@swift.org> wrote:

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to the
standard library's Mirror type a 'summary' property, and the option to
initialize a Mirror with a custom summary. If no custom summary is
provided, the summary would default to the string produced by calling
String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard
library: CustomReflectable, which is publicly exposed and relies on the
conforming type creating a Mirror object, and _Reflectable, which relies on
the conforming type having a companion type conforming to _MirrorType. A
short-term goal is to migrate the standard library's types off the
_Reflectable API and have them use the CustomReflectable API, and changing
dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called
"summary". (This is where e.g. "4 elements" comes from when you dump() an
array.) "summary" is absent from Mirror or any types related to
CustomReflectable. I asked Joe Groff about this and the rationale was that
it was deemed too similar to debugDescription (or String(reflecting: foo))
to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often print
out a description of their children as part of their debugDescription or
description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

dabrahams · 2016-01-06T01:37:01.000Z

Hi Joe,

I respect the choice of the team to use Custom[Debug]StringConvertible in lieu of summary. At the same time, in my opinion the output of dump() has become significantly more difficult to read (c.f. unit tests in Migrate mirrors to use CustomReflectable API, rewrite dump() by austinzheng · Pull Request #838 · apple/swift · GitHub).

Specific examples of readability regressions, please?

Would you and the team be open to exploring alternative solutions that improve the readability of dump() without increasing API surface area?

Sure.

For example, perhaps the reflection machinery itself should have special handling for some of the built-in types. If not, I'll consider this discussion thread complete.

Thanks,
Austin

Getting custom summaries for the common CG types certainly seems reasonable. We'd have to get approval from the appropriate teams at Apple, but I can't see any objections.

Jordan

I believe 'summary' is obsolete, and you're supposed to use Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to the standard library's Mirror type a 'summary' property, and the option to initialize a Mirror with a custom summary. If no custom summary is provided, the summary would default to the string produced by calling String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard library: CustomReflectable, which is publicly exposed and relies on the conforming type creating a Mirror object, and _Reflectable, which relies on the conforming type having a companion type conforming to _MirrorType. A short-term goal is to migrate the standard library's types off the _Reflectable API and have them use the CustomReflectable API, and changing dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called "summary". (This is where e.g. "4 elements" comes from when you dump() an array.) "summary" is absent from Mirror or any types related to CustomReflectable. I asked Joe Groff about this and the rationale was that it was deemed too similar to debugDescription (or String(reflecting: foo)) to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often print out a description of their children as part of their debugDescription or description, redundant when using an API like dump() which provides a structural representation of the children of the subject. In such cases a lighter-weight description (like "3 elements") might be more appropriate to represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible, CustomDebugStringConvertible, Streamable, etc. Having a custom summary for these types customized by the corresponding Mirror would allow for a 'pretty' representation during reflection in lieu of the ugly one generated by the runtime without making more substantial changes to the API which might break third-party code (such as conforming CGRect to any of the aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for major design changes, so this would be a minor transient change to make the API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is meaningful and worthwhile, or if you have any questions.

Best,
Austin
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave

···

On Jan 5, 2016, at 5:28 PM, Austin Zheng via swift-evolution <swift-evolution@swift.org> wrote:
On Tue, Jan 5, 2016 at 3:22 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Austin · 2016-01-06T02:08:36.000Z

Here are a couple of examples I had in mind.

* Arrays (from test/1_stdlib/Runtime.swift:1348), dumping an array with 5
elements:

BEFORE:
▿ 5 elements
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks
    - ClarisWorks: true
       ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard
    - HyperCard: false

AFTER:
▿ [a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true),
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)]
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true)
    - ClarisWorks: true
    ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)
    - HyperCard: false

* Dictionaries (from test/1_stdlib/ReflectionHashing.swift:43):

BEFORE:
▿ 5 key/value pairs
  ▿ [0]: (2 elements)
    - .0: Four
    - .1: 4
  ▿ [1]: (2 elements)
    ...

AFTER:
▿ ["Four": 4, "One": 1, "Two": 2, "Five": 5, "Three": 3]
  ▿ [0]: ("Four", 4)
    - .0: "Four"
    - .1: 4
  ▿ [1]: ("One", 1)
    ...

* Dumping a CGRect (from test/1_stdlib/Reflection_objc.swift):

BEFORE:
(50.0, 60.0, 100.0, 150.0)

AFTER:
__C.CGRect(origin: __C.CGPoint(x: 50.0, y: 60.0), size: __C.CGSize(width:
100.0, height: 150.0))

Let me know if you'd like more, although most are variants on the above.

Best,
Austin

···

On Tue, Jan 5, 2016 at 5:37 PM, Dave Abrahams <dabrahams@apple.com> wrote:

On Jan 5, 2016, at 5:28 PM, Austin Zheng via swift-evolution < > swift-evolution@swift.org> wrote:

Hi Joe,

I respect the choice of the team to use Custom[Debug]StringConvertible in
lieu of summary. At the same time, in my opinion the output of dump() has
become significantly more difficult to read (c.f. unit tests in
Migrate mirrors to use CustomReflectable API, rewrite dump() by austinzheng · Pull Request #838 · apple/swift · GitHub).

Specific examples of readability regressions, please?

Would you and the team be open to exploring alternative solutions that
improve the readability of dump() without increasing API surface area?

Sure.

For example, perhaps the reflection machinery itself should have special
handling for some of the built-in types. If not, I'll consider this
discussion thread complete.

Thanks,
Austin

On Tue, Jan 5, 2016 at 3:22 PM, Jordan Rose <jordan_rose@apple.com> wrote:

Getting custom summaries for the common CG types certainly seems
reasonable. We'd have to get approval from the appropriate teams at Apple,
but I can't see any objections.

Jordan

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution < >> swift-evolution@swift.org> wrote:

I believe 'summary' is obsolete, and you're supposed to use
Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution < >> swift-evolution@swift.org> wrote:

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to
the standard library's Mirror type a 'summary' property, and the option to
initialize a Mirror with a custom summary. If no custom summary is
provided, the summary would default to the string produced by calling
String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard
library: CustomReflectable, which is publicly exposed and relies on the
conforming type creating a Mirror object, and _Reflectable, which relies on
the conforming type having a companion type conforming to _MirrorType. A
short-term goal is to migrate the standard library's types off the
_Reflectable API and have them use the CustomReflectable API, and changing
dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called
"summary". (This is where e.g. "4 elements" comes from when you dump() an
array.) "summary" is absent from Mirror or any types related to
CustomReflectable. I asked Joe Groff about this and the rationale was that
it was deemed too similar to debugDescription (or String(reflecting: foo))
to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often
print out a description of their children as part of their debugDescription
or description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

-Dave

dabrahams · 2016-01-18T08:05:16.000Z

Hey, Austin,

Is this still something we need to discuss, or did it get resolved
somehow?

Thanks,
Dave

···

on Tue Jan 05 2016, Austin Zheng via swift-evolution <swift-evolution-m3FHrko0VLzYtjvyW6yDsg-AT-public.gmane.org> wrote:

Here are a couple of examples I had in mind.

* Arrays (from test/1_stdlib/Runtime.swift:1348), dumping an array with 5
elements:

BEFORE:
▿ 5 elements
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks
    - ClarisWorks: true
       ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard
    - HyperCard: false

AFTER:
▿ [a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true),
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)]
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true)
    - ClarisWorks: true
    ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)
    - HyperCard: false

* Dictionaries (from test/1_stdlib/ReflectionHashing.swift:43):

BEFORE:
▿ 5 key/value pairs
  ▿ [0]: (2 elements)
    - .0: Four
    - .1: 4
  ▿ [1]: (2 elements)
    ...

AFTER:
▿ ["Four": 4, "One": 1, "Two": 2, "Five": 5, "Three": 3]
  ▿ [0]: ("Four", 4)
    - .0: "Four"
    - .1: 4
  ▿ [1]: ("One", 1)
    ...

* Dumping a CGRect (from test/1_stdlib/Reflection_objc.swift):

BEFORE:
(50.0, 60.0, 100.0, 150.0)

AFTER:
__C.CGRect(origin: __C.CGPoint(x: 50.0, y: 60.0), size: __C.CGSize(width:
100.0, height: 150.0))

Let me know if you'd like more, although most are variants on the above.

Best,
Austin

On Tue, Jan 5, 2016 at 5:37 PM, Dave Abrahams <dabrahams@apple.com> wrote:

On Jan 5, 2016, at 5:28 PM, Austin Zheng via swift-evolution < >> swift-evolution@swift.org> wrote:

Hi Joe,

I respect the choice of the team to use Custom[Debug]StringConvertible in
lieu of summary. At the same time, in my opinion the output of dump() has
become significantly more difficult to read (c.f. unit tests in
Migrate mirrors to use CustomReflectable API, rewrite dump() by austinzheng · Pull Request #838 · apple/swift · GitHub).

Specific examples of readability regressions, please?

Would you and the team be open to exploring alternative solutions that
improve the readability of dump() without increasing API surface area?

Sure.

For example, perhaps the reflection machinery itself should have special
handling for some of the built-in types. If not, I'll consider this
discussion thread complete.

Thanks,
Austin

On Tue, Jan 5, 2016 at 3:22 PM, Jordan Rose <jordan_rose@apple.com> wrote:

Getting custom summaries for the common CG types certainly seems
reasonable. We'd have to get approval from the appropriate teams at Apple,
but I can't see any objections.

Jordan

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution < >>> swift-evolution@swift.org> wrote:

I believe 'summary' is obsolete, and you're supposed to use
Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution < >>> swift-evolution@swift.org> wrote:

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to
the standard library's Mirror type a 'summary' property, and the option to
initialize a Mirror with a custom summary. If no custom summary is
provided, the summary would default to the string produced by calling
String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard
library: CustomReflectable, which is publicly exposed and relies on the
conforming type creating a Mirror object, and _Reflectable, which relies on
the conforming type having a companion type conforming to _MirrorType. A
short-term goal is to migrate the standard library's types off the
_Reflectable API and have them use the CustomReflectable API, and changing
dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called
"summary". (This is where e.g. "4 elements" comes from when you dump() an
array.) "summary" is absent from Mirror or any types related to
CustomReflectable. I asked Joe Groff about this and the rationale was that
it was deemed too similar to debugDescription (or String(reflecting: foo))
to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often
print out a description of their children as part of their debugDescription
or description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin

Austin · 2016-01-18T16:57:18.000Z

Hi Dave,

I don't think any progress has been made on this specific issue. Slava made a comment a week ago (https://github.com/apple/swift/pull/838, search "This looks like a QoI regression to me"), but nothing since then.

I'm planning on spending today to work on resolving some of the other issues that were brought up in the PR; maybe we can pick up the conversation again after I update it. Let me know what you prefer.

Best,
Austin

···

On Jan 18, 2016, at 12:05 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Tue Jan 05 2016, Austin Zheng via swift-evolution <swift-evolution-m3FHrko0VLzYtjvyW6yDsg-AT-public.gmane.org <http://swift-evolution-m3fhrko0vlzytjvyw6ydsg-at-public.gmane.org/>> wrote:

Here are a couple of examples I had in mind.

* Arrays (from test/1_stdlib/Runtime.swift:1348), dumping an array with 5
elements:

BEFORE:
▿ 5 elements
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks
   - ClarisWorks: true
      ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard
   - HyperCard: false

AFTER:
▿ [a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true),
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)]
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true)
   - ClarisWorks: true
   ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)
   - HyperCard: false

* Dictionaries (from test/1_stdlib/ReflectionHashing.swift:43):

BEFORE:
▿ 5 key/value pairs
▿ [0]: (2 elements)
   - .0: Four
   - .1: 4
▿ [1]: (2 elements)
   ...

AFTER:
▿ ["Four": 4, "One": 1, "Two": 2, "Five": 5, "Three": 3]
▿ [0]: ("Four", 4)
   - .0: "Four"
   - .1: 4
▿ [1]: ("One", 1)
   ...

* Dumping a CGRect (from test/1_stdlib/Reflection_objc.swift):

BEFORE:
(50.0, 60.0, 100.0, 150.0)

AFTER:
__C.CGRect(origin: __C.CGPoint(x: 50.0, y: 60.0), size: __C.CGSize(width:
100.0, height: 150.0))

Let me know if you'd like more, although most are variants on the above.

Best,
Austin

On Tue, Jan 5, 2016 at 5:37 PM, Dave Abrahams <dabrahams@apple.com> wrote:

On Jan 5, 2016, at 5:28 PM, Austin Zheng via swift-evolution < >>> swift-evolution@swift.org> wrote:

Hi Joe,

I respect the choice of the team to use Custom[Debug]StringConvertible in
lieu of summary. At the same time, in my opinion the output of dump() has
become significantly more difficult to read (c.f. unit tests in
Migrate mirrors to use CustomReflectable API, rewrite dump() by austinzheng · Pull Request #838 · apple/swift · GitHub).

Specific examples of readability regressions, please?

Would you and the team be open to exploring alternative solutions that
improve the readability of dump() without increasing API surface area?

Sure.

For example, perhaps the reflection machinery itself should have special
handling for some of the built-in types. If not, I'll consider this
discussion thread complete.

Thanks,
Austin

On Tue, Jan 5, 2016 at 3:22 PM, Jordan Rose <jordan_rose@apple.com> wrote:

Getting custom summaries for the common CG types certainly seems
reasonable. We'd have to get approval from the appropriate teams at Apple,
but I can't see any objections.

Jordan

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution < >>>> swift-evolution@swift.org> wrote:

I believe 'summary' is obsolete, and you're supposed to use
Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution < >>>> swift-evolution@swift.org> wrote:

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to
the standard library's Mirror type a 'summary' property, and the option to
initialize a Mirror with a custom summary. If no custom summary is
provided, the summary would default to the string produced by calling
String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard
library: CustomReflectable, which is publicly exposed and relies on the
conforming type creating a Mirror object, and _Reflectable, which relies on
the conforming type having a companion type conforming to _MirrorType. A
short-term goal is to migrate the standard library's types off the
_Reflectable API and have them use the CustomReflectable API, and changing
dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called
"summary". (This is where e.g. "4 elements" comes from when you dump() an
array.) "summary" is absent from Mirror or any types related to
CustomReflectable. I asked Joe Groff about this and the rationale was that
it was deemed too similar to debugDescription (or String(reflecting: foo))
to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often
print out a description of their children as part of their debugDescription
or description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin

Hey, Austin,

Is this still something we need to discuss, or did it get resolved
somehow?

Thanks,
Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

dabrahams · 2016-01-18T17:56:39.000Z

Hi Austin, I think we should discuss it here.

First off, dump() was really just added as a proof-of-concept by Joe
Groff when he implemented the original mirror system. We decided to
keep it around because it seemed like it might be useful for somebody,
but one possibility to consider is that it should be retired.

For the rest, see below

Hi Dave,

I don't think any progress has been made on this specific issue. Slava
made a comment a week ago (https://github.com/apple/swift/pull/838
<https://github.com/apple/swift/pull/838>, search "This looks like a
QoI regression to me"), but nothing since then.

I'm planning on spending today to work on resolving some of the other
issues that were brought up in the PR; maybe we can pick up the
conversation again after I update it. Let me know what you prefer.

Best,
Austin

Here are a couple of examples I had in mind.

* Arrays (from test/1_stdlib/Runtime.swift:1348), dumping an array with 5
elements:

BEFORE:
▿ 5 elements
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks
   - ClarisWorks: true
      ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard
   - HyperCard: false

AFTER:
▿ [a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker,
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true),
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)]
- [0]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacWrite
- [1]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.MacPaint
- [2]: a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.FileMaker
▿ [3]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.ClarisWorks(true)
   - ClarisWorks: true
   ▿ [4]:
a.MultiPayloadTagBitsSmallNonGenericEnumWithDefaultMirror.HyperCard(false)
   - HyperCard: false

* Dictionaries (from test/1_stdlib/ReflectionHashing.swift:43):

BEFORE:
▿ 5 key/value pairs
▿ [0]: (2 elements)
   - .0: Four
   - .1: 4
▿ [1]: (2 elements)
   ...

AFTER:
▿ ["Four": 4, "One": 1, "Two": 2, "Five": 5, "Three": 3]
▿ [0]: ("Four", 4)
   - .0: "Four"
   - .1: 4
▿ [1]: ("One", 1)
   ...

* Dumping a CGRect (from test/1_stdlib/Reflection_objc.swift):

BEFORE:
(50.0, 60.0, 100.0, 150.0)

AFTER:
__C.CGRect(origin: __C.CGPoint(x: 50.0, y: 60.0), size: __C.CGSize(width:
100.0, height: 150.0))

Let me know if you'd like more, although most are variants on the above.

Best,
Austin

Hi Joe,

I respect the choice of the team to use Custom[Debug]StringConvertible in
lieu of summary. At the same time, in my opinion the output of dump() has
become significantly more difficult to read (c.f. unit tests in
Migrate mirrors to use CustomReflectable API, rewrite dump() by austinzheng · Pull Request #838 · apple/swift · GitHub).

Specific examples of readability regressions, please?

Would you and the team be open to exploring alternative solutions that
improve the readability of dump() without increasing API surface area?

Sure.

For example, perhaps the reflection machinery itself should have special
handling for some of the built-in types. If not, I'll consider this
discussion thread complete.

Thanks,
Austin

Getting custom summaries for the common CG types certainly seems
reasonable. We'd have to get approval from the appropriate teams at Apple,
but I can't see any objections.

Jordan

I believe 'summary' is obsolete, and you're supposed to use
Custom[Debug]StringConvertible to customize your type's reporting now.

-Joe

Hi all,

I'd like to gauge reaction for a proposal I was considering: adding to
the standard library's Mirror type a 'summary' property, and the option to
initialize a Mirror with a custom summary. If no custom summary is
provided, the summary would default to the string produced by calling
String(reflecting: subject) on the subject at the time of mirror creation.

Some context: right now, there are two APIs for mirrors in the standard
library: CustomReflectable, which is publicly exposed and relies on the
conforming type creating a Mirror object, and _Reflectable, which relies on
the conforming type having a companion type conforming to _MirrorType. A
short-term goal is to migrate the standard library's types off the
_Reflectable API and have them use the CustomReflectable API, and changing
dump() accordingly.

The extant implementation of dump() uses a property on _MirrorType called
"summary". (This is where e.g. "4 elements" comes from when you dump() an
array.) "summary" is absent from Mirror or any types related to
CustomReflectable. I asked Joe Groff about this and the rationale was that
it was deemed too similar to debugDescription (or String(reflecting: foo))
to be worth carrying over.

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often
print out a description of their children as part of their debugDescription
or description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

Agreed, if we keep dump, we ought to recover that functionality somehow.
However, IMO we might be best off synthesizing that text for mirrors
with children by using their DisplayStyle and/or checking their
conformances. It doesn't seem to be worth expanding the protocols for.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I don't think it's worth worrying about breakage from adding a
CustomDebugStringConvertible conformance to CGRect.

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin

Hey, Austin,

Is this still something we need to discuss, or did it get resolved
somehow?

Thanks,
Dave

Thanks,
Dave

···

on Mon Jan 18 2016, Austin Zheng via swift-evolution <swift-evolution-m3FHrko0VLzYtjvyW6yDsg-AT-public.gmane.org> wrote:

On Jan 18, 2016, at 12:05 AM, Dave Abrahams via swift-evolution >> <swift-evolution@swift.org> wrote:
on Tue Jan 05 2016, Austin Zheng via swift-evolution >> <swift-evolution-m3FHrko0VLzYtjvyW6yDsg-AT-public.gmane.org >> <http://swift-evolution-m3fhrko0vlzytjvyw6ydsg-at-public.gmane.org/>> >> wrote:

On Tue, Jan 5, 2016 at 5:37 PM, Dave Abrahams <dabrahams@apple.com> wrote:

On Jan 5, 2016, at 5:28 PM, Austin Zheng via swift-evolution < >>>> swift-evolution@swift.org> wrote:
On Tue, Jan 5, 2016 at 3:22 PM, Jordan Rose <jordan_rose@apple.com> wrote:

On Dec 30, 2015, at 9:55, Joe Groff via swift-evolution < >>>>> swift-evolution@swift.org> wrote:
On Dec 29, 2015, at 10:38 PM, Austin Zheng via swift-evolution < >>>>> swift-evolution@swift.org> wrote:

Austin · 2016-01-19T01:58:03.000Z

Hi Dave,

Thanks for the insight, I think we're on the same page overall. Responses inline.

Hi Austin, I think we should discuss it here.

First off, dump() was really just added as a proof-of-concept by Joe
Groff when he implemented the original mirror system. We decided to
keep it around because it seemed like it might be useful for somebody,
but one possibility to consider is that it should be retired.

Regarding 'dump()', if it was intended 'just' a proof of concept I agree with you - it makes sense to consider retirement. Perhaps designing a replacement (if one should exist at all) should be considered alongside the larger overhaul of the reflection system covered in SR-88. It is used quite extensively in the test suite as a way to quickly validate the structure of instances under test, so removing it would require refactoring many of the unit tests.

For the rest, see below

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often
print out a description of their children as part of their debugDescription
or description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

Agreed, if we keep dump, we ought to recover that functionality somehow.
However, IMO we might be best off synthesizing that text for mirrors
with children by using their DisplayStyle and/or checking their
conformances. It doesn't seem to be worth expanding the protocols for.

Agreed, I think we can easily do it internally for the use cases that are most important (collections, certain built-in types), etc. It's probably out-of-scope for the PR I have out right now (https://bugs.swift.org/browse/SR-88\), but maybe I can sketch an implementation and we can discuss further on swift-dev. (Since it's all internal changes and no public-facing API changes the entire swift-evolution process probably isn't necessary.) LMK if you think that's the best way to proceed.

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I don't think it's worth worrying about breakage from adding a
CustomDebugStringConvertible conformance to CGRect.

Okay, that makes sense. Since it's a public-facing API change, though, I should bring it up on the evolution list and write a proposal. I can do that tonight if that's the most sensible way forward.

Best,
Austin

···

On Jan 18, 2016, at 9:56 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:
on Mon Jan 18 2016, Austin Zheng via swift-evolution <swift-evolution-m3FHrko0VLzYtjvyW6yDsg-AT-public.gmane.org> wrote:

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin

Hey, Austin,

Is this still something we need to discuss, or did it get resolved
somehow?

Thanks,
Dave

Thanks,
Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

dabrahams · 2016-01-19T18:46:17.000Z

Hi Dave,

Thanks for the insight, I think we're on the same page overall. Responses inline.

Hi Austin, I think we should discuss it here.

First off, dump() was really just added as a proof-of-concept by Joe
Groff when he implemented the original mirror system. We decided to
keep it around because it seemed like it might be useful for somebody,
but one possibility to consider is that it should be retired.

Regarding 'dump()', if it was intended 'just' a proof of concept I
agree with you - it makes sense to consider retirement. Perhaps
designing a replacement (if one should exist at all) should be
considered alongside the larger overhaul of the reflection system
covered in SR-88. It is used quite extensively in the test suite as a
way to quickly validate the structure of instances under test, so
removing it would require refactoring many of the unit tests.

Hm.

For the rest, see below

I would like to suggest that there might be a purpose for "summary":

- Types with children, especially container types like arrays, often
print out a description of their children as part of their debugDescription
or description, redundant when using an API like dump() which provides a
structural representation of the children of the subject. In such cases a
lighter-weight description (like "3 elements") might be more appropriate to
represent to the user.

Agreed, if we keep dump, we ought to recover that functionality somehow.
However, IMO we might be best off synthesizing that text for mirrors
with children by using their DisplayStyle and/or checking their
conformances. It doesn't seem to be worth expanding the protocols for.

Agreed, I think we can easily do it internally for the use cases that
are most important (collections, certain built-in types), etc. It's
probably out-of-scope for the PR I have out right now
([SR-88] Remove old mirrors · Issue #42710 · apple/swift · GitHub
<https://bugs.swift.org/browse/SR-88>\), but maybe I can sketch an
implementation and we can discuss further on swift-dev. (Since it's
all internal changes and no public-facing API changes the entire
swift-evolution process probably isn't necessary.) LMK if you think
that's the best way to proceed.

It sounds like a fine plan. You'd have to land it before SR-88 to
keep the tests working, right?

- Certain types like CGRect don't conform to CustomStringConvertible,
CustomDebugStringConvertible, Streamable, etc. Having a custom summary for
these types customized by the corresponding Mirror would allow for a
'pretty' representation during reflection in lieu of the ugly one generated
by the runtime without making more substantial changes to the API which
might break third-party code (such as conforming CGRect to any of the
aforementioned protocols).

I don't think it's worth worrying about breakage from adding a
CustomDebugStringConvertible conformance to CGRect.

Okay, that makes sense. Since it's a public-facing API change, though,
I should bring it up on the evolution list and write a proposal. I can
do that tonight if that's the most sensible way forward.

Sorry, I don't mean to be dense, but what API change are you talking
about making?

···

on Mon Jan 18 2016, Austin Zheng <austinzheng-AT-gmail.com> wrote:

On Jan 18, 2016, at 9:56 AM, Dave Abrahams via swift-evolution >> <swift-evolution@swift.org> wrote:
on Mon Jan 18 2016, Austin Zheng via swift-evolution >> <swift-evolution-m3FHrko0VLzYtjvyW6yDsg-AT-public.gmane.org> wrote:

Best,
Austin

I know that Mirror (and reflection as a whole) are being considered for
major design changes, so this would be a minor transient change to make the
API easier to work with in the meantime.

Please let me know whether or not you think this proposed change is
meaningful and worthwhile, or if you have any questions.

Best,
Austin

Hey, Austin,

Is this still something we need to discuss, or did it get resolved
somehow?

Thanks,
Dave

Thanks,
Dave

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Austin · 2016-01-19T18:52:50.000Z

Hi Dave,

(inline)

(snip)
>
> Agreed, I think we can easily do it internally for the use cases that
> are most important (collections, certain built-in types), etc. It's
> probably out-of-scope for the PR I have out right now
> ([SR-88] Remove old mirrors · Issue #42710 · apple/swift · GitHub
> <https://bugs.swift.org/browse/SR-88>\), but maybe I can sketch an
> implementation and we can discuss further on swift-dev. (Since it's
> all internal changes and no public-facing API changes the entire
> swift-evolution process probably isn't necessary.) LMK if you think
> that's the best way to proceed.

It sounds like a fine plan. You'd have to land it before SR-88 to
keep the tests working, right?

The PR for SR-88 actually has changes to all the tests to keep them passing
without doing any additional work. If I did do the work described above (to
prettify the common use cases), I'd just remove those test changes and keep
the tests the way they look right now.

This means I can either:

1. Land the PR now, and then make the prettifying changes + revert the
tests and commit that as another PR
2. Add the prettifying changes as part of the current PR and delete the
changes I made to the tests (this will make the PR a lot smaller overall;
many of the changes to the test files will disappear)

Let me know if that makes sense.

>>>>>>> - Certain types like CGRect don't conform to
CustomStringConvertible,
>>>>>>> CustomDebugStringConvertible, Streamable, etc. Having a custom
summary for
>>>>>>> these types customized by the corresponding Mirror would allow for
a
>>>>>>> 'pretty' representation during reflection in lieu of the ugly one
generated
>>>>>>> by the runtime without making more substantial changes to the API
which
>>>>>>> might break third-party code (such as conforming CGRect to any of
the
>>>>>>> aforementioned protocols).
>>
>> I don't think it's worth worrying about breakage from adding a
>> CustomDebugStringConvertible conformance to CGRect.
>
> Okay, that makes sense. Since it's a public-facing API change, though,
> I should bring it up on the evolution list and write a proposal. I can
> do that tonight if that's the most sensible way forward.

Sorry, I don't mean to be dense, but what API change are you talking
about making?

No worries. The specific API change I was talking about was adding
CustomDebugStringConvertible conformance to CGRect (and, for completeness,
CGSize and CGPoint). If that doesn't count as a public-facing API change
I'll just add it into my PR tonight.

Best,
Austin

···

On Tue, Jan 19, 2016 at 10:46 AM, Dave Abrahams <dabrahams@apple.com> wrote:

dabrahams · 2016-01-20T00:24:43.000Z

I like #2 for minimizing VCS churn.

···

on Tue Jan 19 2016, Austin Zheng <austinzheng-AT-gmail.com> wrote:

Hi Dave,

(inline)

On Tue, Jan 19, 2016 at 10:46 AM, Dave Abrahams <dabrahams@apple.com> wrote:

(snip)
>
> Agreed, I think we can easily do it internally for the use cases that
> are most important (collections, certain built-in types), etc. It's
> probably out-of-scope for the PR I have out right now
> ([SR-88] Remove old mirrors · Issue #42710 · apple/swift · GitHub
> <https://bugs.swift.org/browse/SR-88>\), but maybe I can sketch an
> implementation and we can discuss further on swift-dev. (Since it's
> all internal changes and no public-facing API changes the entire
> swift-evolution process probably isn't necessary.) LMK if you think
> that's the best way to proceed.

It sounds like a fine plan. You'd have to land it before SR-88 to
keep the tests working, right?

The PR for SR-88 actually has changes to all the tests to keep them passing
without doing any additional work. If I did do the work described above (to
prettify the common use cases), I'd just remove those test changes and keep
the tests the way they look right now.

This means I can either:

1. Land the PR now, and then make the prettifying changes + revert the
tests and commit that as another PR
2. Add the prettifying changes as part of the current PR and delete the
changes I made to the tests (this will make the PR a lot smaller overall;
many of the changes to the test files will disappear)

Let me know if that makes sense.