ModuleInterface: Enable interface verification in more tests #42351
Conversation
|
I'm not done adopting the new |
| // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -swift-version 5 -emit-module-interface-path - -enable-library-evolution %s -experimental-print-full-convention -use-clang-function-types | %FileCheck %s | ||
| // RUN: %empty-directory(%t) | ||
| // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -swift-version 5 -emit-module-interface-path %t/full-convention.swiftinterface -enable-library-evolution %s -experimental-print-full-convention -use-clang-function-types | ||
| // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck-module-from-interface %t/full-convention.swiftinterface |
There was a problem hiding this comment.
This is an example of a test where adopting the new substitution would be awkward but we can still add interface verification to the test.
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
41d861f to
688f48b
Compare
|
This is a great idea👍 |
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
688f48b to
e8ba871
Compare
|
I like the ideas of standard module interface substitutions and verifying a lot more interfaces, but I'm not sure we want to loop the (legacy?) driver into all of these tests. It creates additional ways the tests could fail and it makes reproduction a little more difficult, since you have to ask the driver for the frontend invocations and then figure out which one you want. Would it make sense to instead have two frontend job substitutions, one for emitting the interface and one for verifying it? There'd be a little more to type, but it'd avoid these issues. |
Thanks for raising this @beccadax - I think when I discussed this with the team the thought was that this should be an explicitly supported use case of the legacy driver, but it's true that this creates more potential for extraneous driver behaviors to effect a large number of tests. My main goal is just to validate more interfaces so I don't mind approaching it a different way. @xymus, @nkcsgexi, and @artemcm do you have thoughts on this? |
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
e8ba871 to
e709d96
Compare
…and `%target-swift-typecheck-module-from-interface` lit substitutions and adopt them in some tests that involve emitting a swiftinterface. The substitutions specify the additional flags `-swift-version 5 -enable-library-evolution`. There are a few motivations for adding these substitutions: - Tests that depend on emitted interfaces should generally use flags that are typical for modules that have textual interfaces (e.g. `-enable-library-evolution`). - If a test is intended to produce a valid `swiftinterface` then it should verify that interface. This will help prevent interface printing regressions caused by compiler changes. - Having commonly used substitutions for tests that emit interfaces makes it easy to experiment with compiler flags that might effect interface printing. Resolves rdar://91634358
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
e709d96 to
21e6c36
Compare
|
@swift-ci please test |
…in back-deploy-attr.swift. Simplify the test now that another test exercises deserialization of the attribute from a module.
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
21e6c36 to
8c46b92
Compare
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
8c46b92 to
42ef3af
Compare
…in skip-override-keyword.swift and skip-override-spi.swift. Add CHECK lines to these tests to make their expectations a bit more explicit.
…in inherited-generic-parameters.swift. This revealed that function decls without explicit parameter names can be printed into swiftinterfaces with duplicate synthesized names so update the test to avoid that bug.
…in escape-Type-and-Protocol.swift. Update the test to no longer expect @_hasInitialValue to be emitted since that attribute is intentionally dropped from interfaces when -enable-library-evolution is specified.
…in available-attr-no-collapse.swift. Also, put the input file contents directly in the test file instead of `RUN: echo` lines to make the test a bit easier to understand.
… in emit-interface-macos-canonical-version.swift. Move the test to the ModuleInterface directory to colocate it with other swiftinterface generation tests and rename it to availability-macos-canonical-version.swift. Remove an extraneous -experimental-skip-non-inlinable-function-bodies flag that seemed like it was copypasta.
…in access-filter.swift. Fix an error caused by conforming to a @usableFromInline protocol using a non-UFI member (this was diagnosed as a warning in Swift 4 and became an error in Swift 5). Also, remove @_hasInitialValue from the expted swiftinterface contents since that attribute isn't emitted for types in resilient modules.
…in preserve-type-repr.swift. The test was originally written assuming that `CHECK:` lines are always matched by FileCheck even when alternate prefixes are specified via `--check-prefix`, so it contained some incorrect expectations that needed to be updated.
…in synthesized.swift. Update the test to expect output for a resilient framework by removing `@inlinable` attributes from a few decls.
…ons pervasively in tests where they can be adopted by simply updating or adding a few RUN: lines.
tshortli
force-pushed
the
adopt-interface-verification-in-more-tests
branch
from
42ef3af to
747f286
Compare
|
@swift-ci please test |
Introduce a
%target-swiftc-emit-module-interfacelit substitution and adopt it in some tests that involve emitting a swiftinterface. The substitution specifies the additional flags-swift-version 5 -enable-library-evolution -verify-emitted-module-interface. There are a few motivations:-enable-library-evolution).swiftinterfacethen it should verify that interface. This will help prevent interface printing regressions caused by compiler changes.Resolves rdar://91634358