107 lines of "dead" code in ConstraintSystem::performMemberLookup?

In ConstraintSystem::performMemberLookup(), constructors with “simple” names have a dedicated lookup path. In contrast, constructors with compound names are handled by the normal lookup. If I delete this code and let the normal lookup path handle both simple and compound named constructors, I find that all 10,214 validation tests pass on my machine (albeit with slightly different error messages in three test files).

Is the test suite missing a test for this code path and if so, what? Or should it be scheduled for deletion after identical error messages can be generated by the normal lookup path?

Dave

In ConstraintSystem::performMemberLookup(), constructors with “simple” names have a dedicated lookup path. In contrast, constructors with compound names are handled by the normal lookup. If I delete this code and let the normal lookup path handle both simple and compound named constructors, I find that all 10,214 validation tests pass on my machine (albeit with slightly different error messages in three test files).

I would suggest running the source compatibility test suite also (see “pull request testing” in https://swift.org/source-compatibility/\), but it is quite possible the code is indeed unnecessary.

Is the test suite missing a test for this code path and if so, what? Or should it be scheduled for deletion after identical error messages can be generated by the normal lookup path?

Are the new error messages worse or just different? If the latter there’s really no requirement to keep them identical.

Slava

···

On Aug 8, 2017, at 3:34 PM, David Zarzycki via swift-dev <swift-dev@swift.org> wrote:

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

I see you’re two steps ahead of me already: Simplify performMemberLookup() by davezarzycki · Pull Request #11397 · apple/swift · GitHub

This looks good, nice catch!

Slava

···

On Aug 8, 2017, at 8:45 PM, Slava Pestov <spestov@apple.com> wrote:

On Aug 8, 2017, at 3:34 PM, David Zarzycki via swift-dev <swift-dev@swift.org> wrote:

In ConstraintSystem::performMemberLookup(), constructors with “simple” names have a dedicated lookup path. In contrast, constructors with compound names are handled by the normal lookup. If I delete this code and let the normal lookup path handle both simple and compound named constructors, I find that all 10,214 validation tests pass on my machine (albeit with slightly different error messages in three test files).

I would suggest running the source compatibility test suite also (see “pull request testing” in https://swift.org/source-compatibility/\), but it is quite possible the code is indeed unnecessary.

Is the test suite missing a test for this code path and if so, what? Or should it be scheduled for deletion after identical error messages can be generated by the normal lookup path?

Are the new error messages worse or just different? If the latter there’s really no requirement to keep them identical.

Slava

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

Hi Slava,

Thanks! I appreciate the suggestion to run tests, but what I’ve tried in the past, @swift-ci ignored me. Is there a whitelist for who can request that tests be run? That would make sense from a security perspective.

Dave

···

On Aug 8, 2017, at 23:48, Slava Pestov <spestov@apple.com> wrote:

I see you’re two steps ahead of me already: Simplify performMemberLookup() by davezarzycki · Pull Request #11397 · apple/swift · GitHub

This looks good, nice catch!

Slava

On Aug 8, 2017, at 8:45 PM, Slava Pestov <spestov@apple.com <mailto:spestov@apple.com>> wrote:

On Aug 8, 2017, at 3:34 PM, David Zarzycki via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

In ConstraintSystem::performMemberLookup(), constructors with “simple” names have a dedicated lookup path. In contrast, constructors with compound names are handled by the normal lookup. If I delete this code and let the normal lookup path handle both simple and compound named constructors, I find that all 10,214 validation tests pass on my machine (albeit with slightly different error messages in three test files).

I would suggest running the source compatibility test suite also (see “pull request testing” in https://swift.org/source-compatibility/\), but it is quite possible the code is indeed unnecessary.

Is the test suite missing a test for this code path and if so, what? Or should it be scheduled for deletion after identical error messages can be generated by the normal lookup path?

Are the new error messages worse or just different? If the latter there’s really no requirement to keep them identical.

Slava

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

I think there are docs on this in the main README or in a CONTRIBUTING file.

Mishal (+CC) knows for sure.

Michael

···

On Aug 9, 2017, at 5:41 AM, David Zarzycki via swift-dev <swift-dev@swift.org> wrote:

Hi Slava,

Thanks! I appreciate the suggestion to run tests, but what I’ve tried in the past, @swift-ci ignored me. Is there a whitelist for who can request that tests be run? That would make sense from a security perspective.

Dave

On Aug 8, 2017, at 23:48, Slava Pestov <spestov@apple.com <mailto:spestov@apple.com>> wrote:

I see you’re two steps ahead of me already: Simplify performMemberLookup() by davezarzycki · Pull Request #11397 · apple/swift · GitHub

This looks good, nice catch!

Slava

On Aug 8, 2017, at 8:45 PM, Slava Pestov <spestov@apple.com <mailto:spestov@apple.com>> wrote:

On Aug 8, 2017, at 3:34 PM, David Zarzycki via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

In ConstraintSystem::performMemberLookup(), constructors with “simple” names have a dedicated lookup path. In contrast, constructors with compound names are handled by the normal lookup. If I delete this code and let the normal lookup path handle both simple and compound named constructors, I find that all 10,214 validation tests pass on my machine (albeit with slightly different error messages in three test files).

I would suggest running the source compatibility test suite also (see “pull request testing” in https://swift.org/source-compatibility/\), but it is quite possible the code is indeed unnecessary.

Is the test suite missing a test for this code path and if so, what? Or should it be scheduled for deletion after identical error messages can be generated by the normal lookup path?

Are the new error messages worse or just different? If the latter there’s really no requirement to keep them identical.

Slava

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

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