[Explicit Module Builds] Prevent SerializedModuleLoader from running in Explicit Module Build mode. #32903

artemcm · 2020-07-15T17:20:48Z

In order to avoid accidentally implicitly loading modules that are expected but were not provided as explicit inputs.

Rename SerializedModuleLoader into ImplicitSerializedModuleLoader
Use either ImplicitSerializedModuleLoader or ExplicitSwiftModuleLoader for loading of partial modules, depending on whether we are in Explicit Module Build or Implicit Module Build mode.

artemcm · 2020-07-15T17:22:41Z

@swift-ci test

nkcsgexi

Makes sense to me! Only a minor suggestion.

lib/Frontend/Frontend.cpp

artemcm · 2020-07-15T18:02:18Z

@swift-ci test

swift-ci · 2020-07-15T18:05:08Z

Build failed
Swift Test Linux Platform
Git Sha - 7cefbf9419b65142c85cb9055dfd1265ed14ef3d

swift-ci · 2020-07-15T18:05:43Z

Build failed
Swift Test OS X Platform
Git Sha - 7cefbf9419b65142c85cb9055dfd1265ed14ef3d

swift-ci · 2020-07-15T18:06:25Z

Build failed
Swift Test OS X Platform
Git Sha - 7cefbf9419b65142c85cb9055dfd1265ed14ef3d

swift-ci · 2020-07-15T19:06:20Z

Build failed
Swift Test Linux Platform
Git Sha - 7cefbf9419b65142c85cb9055dfd1265ed14ef3d

artemcm · 2020-07-15T21:14:41Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please test

swift-ci · 2020-07-15T22:19:59Z

Build failed
Swift Test Linux Platform
Git Sha - 62286f40b22c3f25e9d7f4d714ed78831db36493

swift-ci · 2020-07-15T22:52:52Z

Build failed
Swift Test OS X Platform
Git Sha - 62286f40b22c3f25e9d7f4d714ed78831db36493

artemcm · 2020-07-16T16:55:34Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please test

swift-ci · 2020-07-16T16:58:32Z

Build failed
Swift Test Linux Platform
Git Sha - 62286f40b22c3f25e9d7f4d714ed78831db36493

swift-ci · 2020-07-16T16:59:32Z

Build failed
Swift Test OS X Platform
Git Sha - 62286f40b22c3f25e9d7f4d714ed78831db36493

artemcm · 2020-07-16T18:07:28Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please test

swift-ci · 2020-07-16T19:15:32Z

Build failed
Swift Test Linux Platform
Git Sha - d54460d87055ec33bfb594590ec3a1e34b5c1a50

swift-ci · 2020-07-16T19:22:38Z

Build failed
Swift Test OS X Platform
Git Sha - d54460d87055ec33bfb594590ec3a1e34b5c1a50

artemcm · 2020-07-17T03:12:41Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please test

swift-ci · 2020-07-17T03:15:22Z

Build failed
Swift Test Linux Platform
Git Sha - d54460d87055ec33bfb594590ec3a1e34b5c1a50

swift-ci · 2020-07-17T03:16:08Z

Build failed
Swift Test OS X Platform
Git Sha - d54460d87055ec33bfb594590ec3a1e34b5c1a50

artemcm · 2020-07-17T19:57:36Z

@swift-ci please test

swift-ci · 2020-07-17T19:59:50Z

Build failed
Swift Test Linux Platform
Git Sha - 5836a7a69e4cf3fbb2c4e2966f2b8456a8d4992c

swift-ci · 2020-07-17T20:01:09Z

Build failed
Swift Test OS X Platform
Git Sha - 5836a7a69e4cf3fbb2c4e2966f2b8456a8d4992c

artemcm · 2020-07-21T03:29:40Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please smoke test

swift-ci · 2020-07-21T03:32:23Z

Build failed
Swift Test Linux Platform
Git Sha - 4f8cd28e266e4900da687790328017a95838c858

swift-ci · 2020-07-21T03:33:08Z

Build failed
Swift Test OS X Platform
Git Sha - 4f8cd28e266e4900da687790328017a95838c858

artemcm · 2020-07-21T15:57:23Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please smoke test

artemcm · 2020-07-21T19:52:00Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please smoke test

…in Explicit Module Build mode. In order to avoid accidentally implicitly loading modules that are expected but were not provided as explicit inputs. - Use either SerializedModuleLoader or ExplicitSwiftModuleLoader for loading of partial modules, depending on whether we are in Explicit Module Build or Implicit Module Build mode.

artemcm · 2020-07-22T16:03:39Z

Please test with following PR:
swiftlang/llvm-project#1465

@swift-ci Please smoke test

compnerd · 2020-07-22T23:21:47Z

This seems to have caused a regression on the Windows builder:
https://ci-external.swift.org/job/oss-swift-windows-x86_64-vs2019/2188/

Could you please take a look @artemcm?

CC: @shahmishal - seems that the CI didn't trigger?

artemcm · 2020-07-22T23:23:51Z

This seems to have caused a regression on the Windows builder:
https://ci-external.swift.org/job/oss-swift-windows-x86_64-vs2019/2188/

Could you please take a look @artemcm?

CC: @shahmishal - seems that the CI didn't trigger?

@compnerd , what is the path structure for Swift.swiftmodule on Windows? I think the change to lit.cfg here just needs to be updated with a windows special case.

compnerd · 2020-07-22T23:25:30Z

It is identical to Linux, just with windows instead of linux in the resource directory

artemcm · 2020-07-22T23:28:05Z

It is identical to Linux, just with windows instead of linux in the resource directory

Got it, I'll prepare a patch right away.

Repair the Windows build after swiftlang#32903

In swiftlang#32903, a substitution pseudovariable was introduced to refer to the path containing Swift.swiftmodule. Since builds on Linux put this file in a path with a subdirectory named after the architecture name -- and presumably builds on mac OS do not, a conditional was required to distinguish these two cases. However, builds on OpenBSD for example also put Swift.swiftmodule in a similar subdirectory like Linux. Thus, invert the sense of this conditional to encompass both Linux, OpenBSD, and make mac OS the specific case, which it is here.

varungandhi-apple · 2020-07-26T06:33:38Z

lib/Frontend/Frontend.cpp

-//    [Note: ModuleInterfaceLoader-defer-to-SerializedModuleLoader]), which
+//    [Note: ModuleInterfaceLoader-defer-to-ImplicitSerializedModuleLoader]), which
 //    is interpreted by the overarching loader as a command to use the
-//    SerializedModuleLoader. If we failed to find a .swiftmodule, this falls
+//    ImplicitSerializedModuleLoader. If we failed to find a .swiftmodule, this falls


Please be careful about updating comments. There are a couple of issues here:

This is exceeding the line length limit now.

This broke a reference to the note, since the reference now has a new name but the note's name in ModuleInterfaceLoader.cpp is not changed in this PR.

I'm going to fix these two issues in an upcoming PR, it's not a big deal; just something to be careful about.

PR with fix: #33124

artemcm requested a review from nkcsgexi July 15, 2020 17:20

artemcm force-pushed the NoImplicitWhenExplicit branch from a9910d6 to 7cefbf9 Compare July 15, 2020 17:21

nkcsgexi approved these changes Jul 15, 2020

View reviewed changes

lib/Frontend/Frontend.cpp Outdated Show resolved Hide resolved

artemcm force-pushed the NoImplicitWhenExplicit branch from 7cefbf9 to 62286f4 Compare July 15, 2020 18:01

artemcm mentioned this pull request Jul 15, 2020

Rename SerializedModuleLoader to match Swift compiler's new API. swiftlang/llvm-project#1465

Merged

artemcm force-pushed the NoImplicitWhenExplicit branch from 62286f4 to d54460d Compare July 16, 2020 16:54

artemcm force-pushed the NoImplicitWhenExplicit branch 2 times, most recently from 3877689 to 5836a7a Compare July 17, 2020 03:04

artemcm force-pushed the NoImplicitWhenExplicit branch from 5836a7a to 8cbba04 Compare July 17, 2020 19:57

artemcm force-pushed the NoImplicitWhenExplicit branch from 9c294fd to 7fe842e Compare July 21, 2020 15:57

artemcm force-pushed the NoImplicitWhenExplicit branch from 7fe842e to c1da91d Compare July 21, 2020 19:46

artemcm force-pushed the NoImplicitWhenExplicit branch from c1da91d to 140fd73 Compare July 22, 2020 16:01

artemcm merged commit 95c8391 into swiftlang:master Jul 22, 2020

artemcm mentioned this pull request Jul 22, 2020

[Windows] Fix windows breakage caused by lit.cfg change #33062

Merged

3405691582 mentioned this pull request Jul 23, 2020

[test] Invert stdlib_dir conditional sense. #33065

Merged

compnerd mentioned this pull request Jul 23, 2020

repair build after #32903 #33066

Closed

compnerd added a commit to compnerd/apple-swift that referenced this pull request Jul 23, 2020

repair build after swiftlang#32903

be49f44

Repair the Windows build after swiftlang#32903

varungandhi-apple reviewed Jul 26, 2020

View reviewed changes

kastiglione mentioned this pull request Dec 2, 2021

Allow -explicit-swift-module-map-file without full explicit module build #39887

Merged

Uh oh!

[Explicit Module Builds] Prevent SerializedModuleLoader from running in Explicit Module Build mode. #32903

[Explicit Module Builds] Prevent SerializedModuleLoader from running in Explicit Module Build mode. #32903

Uh oh!

Conversation

artemcm commented Jul 15, 2020

Uh oh!

artemcm commented Jul 15, 2020

Uh oh!

nkcsgexi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

artemcm commented Jul 15, 2020

Uh oh!

swift-ci commented Jul 15, 2020

Uh oh!

swift-ci commented Jul 15, 2020

Uh oh!

swift-ci commented Jul 15, 2020

Uh oh!

swift-ci commented Jul 15, 2020

Uh oh!

artemcm commented Jul 15, 2020

Uh oh!

swift-ci commented Jul 15, 2020

Uh oh!

swift-ci commented Jul 15, 2020

Uh oh!

artemcm commented Jul 16, 2020

Uh oh!

swift-ci commented Jul 16, 2020

Uh oh!

swift-ci commented Jul 16, 2020

Uh oh!

artemcm commented Jul 16, 2020

Uh oh!

swift-ci commented Jul 16, 2020

Uh oh!

swift-ci commented Jul 16, 2020

Uh oh!

artemcm commented Jul 17, 2020

Uh oh!

swift-ci commented Jul 17, 2020

Uh oh!

swift-ci commented Jul 17, 2020

Uh oh!

artemcm commented Jul 17, 2020

Uh oh!

swift-ci commented Jul 17, 2020

Uh oh!

swift-ci commented Jul 17, 2020

Uh oh!

artemcm commented Jul 21, 2020

Uh oh!

swift-ci commented Jul 21, 2020

Uh oh!

swift-ci commented Jul 21, 2020

Uh oh!

artemcm commented Jul 21, 2020

Uh oh!

artemcm commented Jul 21, 2020

Uh oh!

artemcm commented Jul 22, 2020

Uh oh!

compnerd commented Jul 22, 2020

Uh oh!

artemcm commented Jul 22, 2020

Uh oh!

compnerd commented Jul 22, 2020

Uh oh!

artemcm commented Jul 22, 2020

Uh oh!

varungandhi-apple Jul 26, 2020

Choose a reason for hiding this comment

Uh oh!

varungandhi-apple Jul 26, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers