Add "-swift-version <n>" that sets LangOpts.EffectiveLanguageVersion. #4826

graydon · 2016-09-16T04:22:58Z

This flag switches the "effective language version" of the compiler,
at least to any version supported (as of this change: 3 <= x <= 4).

At the moment nothing uses it except the language version build
configuration statements (#if swift(...)) and various other places
that report, encode, or otherwise check version numbers.

In the future, it's intended as scaffolding for backwards compatibility.

Resolves SR-2582.

slavapestov · 2016-09-16T04:51:15Z

@swift-ci Please smoke test

DougGregor · 2016-09-16T16:16:33Z

include/swift/Basic/LangOptions.h

@@ -41,6 +42,9 @@ namespace swift {
    /// Language features
    ///

+    /// \brief User-overridable language version to compile for.
+    version::Version EffectiveLanguageVersion = version::Version::getCurrentLanguageVersion();


Can we put this into ModuleDecl? It's plausible that, e.g., the optimizer might want to have multiple modules within the same ASTContext, and those modules might be built with different language modes.

I'm really hoping we don't need to know that at a module level, and putting it in ModuleDecl means it's not really possible to get at it from the importer.

(I do not feel qualified to judge here -- consensus?)

Ah, the Clang importer basically makes the scenario I'm describing unworkable, because there's only one and it has to pick a language mode. So, that means EffectiveLanguageVersion should stay in LangOptions. Thanks, @jrose-apple

DougGregor · 2016-09-16T16:20:48Z

lib/Driver/Driver.cpp

+      // follows.
+      versionValid = (value->getValue(scratch)
+                      == version::getSwiftFullVersion(
+                        swift::version::Version::getCurrentLanguageVersion()));


version:: should suffice instead of swift::version::.

DougGregor · 2016-09-16T16:24:34Z

lib/Basic/Version.cpp

+  // any version starting with major number 3 or 4.
+  return Components.size() > 0 &&
+    (Components[0] == 3 ||
+     Components[0] == 4);


I think we should reject "3.2", "4.1", etc. dialects that don't exist, and whitelist only those dialects that are explicitly supported.

Ok. I put in support for 4.x as a sort of cheap means to enable testing that -swift-version actually does change the version (since current == 3.0 already) but I will change this to a shorter whitelist and weaken the test to just confirm that -swift-version 3 keeps working when we do bump the language version.

DougGregor · 2016-09-16T16:30:55Z

include/swift/AST/IRGenOptions.h

@@ -67,6 +68,9 @@ class IRGenOptions {
  std::vector<std::string> OutputFilenames;
  std::string ModuleName;

+  /// \brief User-overridable language version to compile for.
+  version::Version EffectiveLanguageVersion = version::Version::getCurrentLanguageVersion();
+


I don't think you need this; the information is always available (see below).

Ah, thinko -- I was looking at IRGenOptions and for some reason didn't notice IRGenModule was also in scope for this usage.

DougGregor · 2016-09-16T16:31:54Z

lib/IRGen/IRGenDebugInfo.cpp

-  std::tie(Major, Minor) = version::getSwiftNumericVersion();
-  unsigned MajorRuntimeVersion = Major;
+
+  assert(!Opts.EffectiveLanguageVersion.empty());


I don't know what the version is supposed to mean here... is it the compiler version (because clients want to make sure they're working with a compatible compiler) or is it the language dialect? I suspect the former is actually more useful for a debugger.

... but if we do need to use the EffectiveLanguageVersion, you can get it off the corresponding Swift module or ASTContext.

As far as "what it means", it appears to thread through to an attribute on the CU DIE called dwarf::DW_AT_APPLE_major_runtime_vers which, as far as google can tell, nobody in the world actually uses. It showed up in 2009 in LLVM (apple/swift-llvm@13319ce) and it looks like possibly it was intended to be related to LLDB logic around ObjC runtime versions, but LLDB currently just sniffs the existence of a "telltale section" (https://github.com/apple/swift-lldb/blob/master/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp#L401-L409) and uses it to switch between V1 and V2.

In any case if we're emitting 3 for it now -- as we seem to be doing as of swift 3.0 -- and if ObjC doesn't have a runtime version 3, we're probably already doing wrong here. I'll add a helper function that picks a runtime version to match the current language version, explicitly.

@gparker42?

We are using the DW_AT_APPLE_runtime_version in the DW_LANG_Swift compile unit to encode the Swift runtime version (actually the swift compiler version because they are thus far identical, cf. rdar://problem/18729762).

The Objective-C runtime version can be found in the second, DW_LANG_ObjC compile unit that is also part of each object file generated by swiftc.

@adrian-prantl Oh, weird, I wonder why I thought this had to do with the associated ObjC runtime. Ok, that makes sense; reverting that change then. Thanks!

Yes, my understanding is that we always want to emit the current swift runtime version here, even if the source the object was compiled from had a lower effective language version.

DougGregor · 2016-09-16T16:32:47Z

lib/PrintAsObjC/PrintAsObjC.cpp

@@ -2037,7 +2037,8 @@ class ModuleWriter {
  }

  void writePrologue(raw_ostream &out) {
-    out << "// Generated by " << version::getSwiftFullVersion() << "\n"
+    out << "// Generated by " << version::getSwiftFullVersion(
+      M.getASTContext().LangOpts.EffectiveLanguageVersion) << "\n"


I flip-flopped a few times, but now I agree: the printed headers from "Swift 3 mode" should be equivalent to what the Swift 3 compiler produced.

Does this mean you want it to be something other than getSwiftFullVersion? Like obscure the fact that it came from a newer compiler altogether?

Using getFullSwiftVersion() is reasonable. I was convincing myself that we wanted '3.0' in there when emulating 3.0, but that seems like the right idea.

jrose-apple · 2016-09-16T17:52:56Z

lib/Serialization/Serialization.cpp

    llvm::raw_svector_ostream versionString(versionStringBuf);
-    versionString << Version::getCurrentLanguageVersion();
+    versionString << version;


This is used in diagnostics to describe module format mismatches, so I think we have no choice but to print the new version. (Or use a more complicated string.)

Well, it does put the full version string after the '/' in the header; I saw (and intended to preserve) the short bit at the beginning is used to detect format mismatches, but I thought that was the point here: that compiling in -swift-version 3 will produce a version-3-compatible module, and that we're going to conditionalize any evolution of the module format to being "in later effective versions".

Perhaps I'm misunderstanding?

Currently, only the short version string is used for diagnostics; actual mismatches are detected using the serialization's VERSION_MAJOR/VERSION_MINOR (itself designed for a stable serialization format that we haven't gotten to yet). The intent is that if the short diagnostic string has changed, we can offer a better diagnostic than just "the module is too old". The full string is mostly just so that we can run strings on a swiftmodule and see where it came from.

Then I'm .. perhaps even more confused here. If the short version is intended for diagnostic output -- of the form "sorry, this is a version <m.n> module!" -- then I would think it should be set to the effective language used to build that module, no? I.e. the way I have it set in this proposal.

I'm happy to change this if I'm misunderstanding, I'm just not sure what to change it to, or why.

I think it's "I can't load this module because it was built with the Swift %0 compiler", not "I can't load this module because it only supports Swift %0 (as a language)". We are planning to support "Swift 3 modules" in Swift 4, but only if they were built with the Swift 4 compiler.

We can tweak the diagnostic too to make that more clear, now that we're planning on having one compiler with two language modes.

Ah ok, setting back to just emit the compiler-associated "current" version in the short string, to help the user identify the cause of the mismatch ("wrong compiler" more than "wrong language version")

jrose-apple · 2016-09-17T00:31:37Z

test/Parse/ConditionalCompilation/language_version_explicit.swift


-#if swift(>=4)
+#if swift(>=3)


This would pass whether or not the -swift-version is set, no?

Yeah, based on Doug's preference for making an explicit whitelist (above) I can't really make a test that tests anything useful yet; you can only specify version 3, which is what the compiler is anyways. When there's more than one real version on the whiltelist, this test can be updated to be more interesting.

DougGregor

I'm okay with this.

DougGregor · 2016-09-20T03:20:47Z

lib/IRGen/IRGenDebugInfo.cpp

+  // dwarf::DW_AT_APPLE_major_runtime_vers) that's appropriate for the effective
+  // language version in effect for the current compilation unit.
+  assert(!IGM.Context.LangOpts.EffectiveLanguageVersion.empty());
+  if (IGM.Context.LangOpts.EffectiveLanguageVersion[0] < 2) {


This is still weird, because Swift only supports the 'nonfragile' Objective-C ABI (i.e., 2), but I guess this maintains the old behavior.

I don't think this was the ObjC version; I think Clang sets that. @gparker42?

Oh, this is debug info. @adrian-prantl?

This flag switches the "effective language version" of the compiler, at least to any version supported (as of this change: "3" or "3.0"). At the moment nothing uses it except the language version build configuration statements (#if swift(...)) and various other places that report, encode, or otherwise check version numbers. In the future, it's intended as scaffolding for backwards compatibility. Fixes SR-2582

graydon · 2016-09-20T22:20:32Z

@swift-ci Please test and merge

graydon · 2016-09-20T22:25:30Z

@swift-ci Please test and merge

DougGregor requested changes Sep 16, 2016

View reviewed changes

jrose-apple reviewed Sep 16, 2016

View reviewed changes

jrose-apple reviewed Sep 17, 2016

View reviewed changes

DougGregor approved these changes Sep 20, 2016

View reviewed changes

graydon force-pushed the SR-2582-swift-version branch 2 times, most recently from 47c9637 to 70e3705 Compare September 20, 2016 19:15

graydon force-pushed the SR-2582-swift-version branch from 70e3705 to 8970d44 Compare September 20, 2016 22:19

swift-ci merged commit ea5aaa6 into swiftlang:master Sep 20, 2016

graydon deleted the SR-2582-swift-version branch September 21, 2016 00:33

graydon mentioned this pull request Sep 23, 2016

SR-2709: @available(swift, ...) #4985

Merged

benrimmington mentioned this pull request Sep 24, 2016

SE-0140 and SE-0110 have been implemented swiftlang/swift-evolution#532

Merged

swift-ci mentioned this pull request Sep 21, 2016

[SR-2582] Add -swift-version command line flag #45187

Closed


		#if swift(>=4)
		#if swift(>=3)

Add "-swift-version <n>" that sets LangOpts.EffectiveLanguageVersion. #4826

Add "-swift-version <n>" that sets LangOpts.EffectiveLanguageVersion. #4826

Uh oh!

Conversation

graydon commented Sep 16, 2016

Uh oh!

slavapestov commented Sep 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor Sep 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graydon Sep 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graydon commented Sep 20, 2016

Uh oh!

graydon commented Sep 20, 2016

DougGregor Sep 16, 2016 •

edited

Loading

graydon Sep 17, 2016 •

edited

Loading