Merge pull request #78642 from tshortli/warn-unrecognized-language-feature

Frontend: Implement optional parsing diagnostics for enabled language features

Author: tshortli

include/swift/AST/DiagnosticGroups.def

@@ -26,6 +26,7 @@ GROUP(DeprecatedDeclaration, "DeprecatedDeclaration.md")
 GROUP(Unsafe, "Unsafe.md")
 GROUP(UnknownWarningGroup, "UnknownWarningGroup.md")
 GROUP(PreconcurrencyImport, "PreconcurrencyImport.md")
+GROUP(StrictLanguageFeatures, "StrictLanguageFeatures.md")
 
 #define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
 #include "swift/AST/DefineDiagnosticGroupsMacros.h"

include/swift/AST/DiagnosticsFrontend.def

@@ -37,8 +37,18 @@ ERROR(error_unsupported_target_arch, none,
       "unsupported target architecture: '%0'", (StringRef))
 
 WARNING(warning_upcoming_feature_on_by_default, none,
-      "upcoming feature '%0' is already enabled as of Swift version %1",
-      (StringRef, unsigned))
+        "upcoming feature '%0' is already enabled as of Swift version %1",
+        (StringRef, unsigned))
+
+GROUPED_WARNING(unrecognized_feature, StrictLanguageFeatures, DefaultIgnore,
+                "'%0' is not a recognized "
+                "%select{experimental|upcoming}1 feature",
+                (StringRef, bool))
+
+GROUPED_WARNING(feature_not_experimental, StrictLanguageFeatures, DefaultIgnore,
+                "'%0' is not an experimental feature, "
+                "use -%select{disable|enable}1-upcoming-feature instead",
+                (StringRef, bool))
 
 ERROR(error_unknown_library_level, none,
       "unknown library level '%0', "

include/swift/Frontend/Frontend.h

@@ -116,12 +116,17 @@ class CompilerInvocation {
 public:
   CompilerInvocation();
 
-  /// Initializes the compiler invocation for the list of arguments.
+  /// Initializes the compiler invocation and diagnostic engine for the list of
+  /// arguments.
   ///
   /// All parsing should be additive, i.e. options should not be reset to their
   /// default values given the /absence/ of a flag. This is because \c parseArgs
   /// may be used to modify an already partially configured invocation.
   ///
+  /// As a side-effect of parsing, the diagnostic engine will be configured with
+  /// the options specified by the parsed arguments. This ensures that the
+  /// arguments can effect the behavior of diagnostics emitted during parsing.
+  ///
   /// Any configuration files loaded as a result of parsing arguments will be
   /// stored in \p ConfigurationFileBuffers, if non-null. The contents of these
   /// buffers should \e not be interpreted by the caller; they are only present
@@ -160,6 +165,9 @@ class CompilerInvocation {
                               StringRef SDKPath,
                               StringRef ResourceDir);
 
+  /// Configures the diagnostic engine for the invocation's options.
+  void setUpDiagnosticEngine(DiagnosticEngine &diags);
+
   void setTargetTriple(const llvm::Triple &Triple);
   void setTargetTriple(StringRef Triple);
 

lib/Frontend/CompilerInvocation.cpp

@@ -20,6 +20,7 @@
 #include "swift/Basic/Assertions.h"
 #include "swift/Basic/Feature.h"
 #include "swift/Basic/Platform.h"
+#include "swift/Basic/Version.h"
 #include "swift/Option/Options.h"
 #include "swift/Option/SanitizerOptions.h"
 #include "swift/Parse/Lexer.h"
@@ -92,20 +93,6 @@ void CompilerInvocation::setMainExecutablePath(StringRef Path) {
   llvm::sys::path::remove_filename(clangPath);
   llvm::sys::path::append(clangPath, "clang");
   ClangImporterOpts.clangPath = std::string(clangPath);
-
-  llvm::SmallString<128> DiagnosticDocsPath(Path);
-  llvm::sys::path::remove_filename(DiagnosticDocsPath); // Remove /swift
-  llvm::sys::path::remove_filename(DiagnosticDocsPath); // Remove /bin
-  llvm::sys::path::append(DiagnosticDocsPath, "share", "doc", "swift",
-                          "diagnostics");
-  DiagnosticOpts.DiagnosticDocumentationPath = std::string(DiagnosticDocsPath.str());
-
-  // Compute the path to the diagnostic translations in the toolchain/build.
-  llvm::SmallString<128> DiagnosticMessagesDir(Path);
-  llvm::sys::path::remove_filename(DiagnosticMessagesDir); // Remove /swift
-  llvm::sys::path::remove_filename(DiagnosticMessagesDir); // Remove /bin
-  llvm::sys::path::append(DiagnosticMessagesDir, "share", "swift", "diagnostics");
-  DiagnosticOpts.LocalizationPath = std::string(DiagnosticMessagesDir.str());
 }
 
 static std::string
@@ -758,6 +745,8 @@ static bool ParseEnabledFeatureArgs(LangOptions &Opts, ArgList &Args,
     auto &option = A->getOption();
     StringRef value = A->getValue();
     bool enableUpcoming = option.matches(OPT_enable_upcoming_feature);
+    bool isUpcomingFlag =
+        enableUpcoming || option.matches(OPT_disable_upcoming_feature);
     bool enableFeature =
         enableUpcoming || option.matches(OPT_enable_experimental_feature);
 
@@ -770,20 +759,28 @@ static bool ParseEnabledFeatureArgs(LangOptions &Opts, ArgList &Args,
       continue;
     }
 
-    // If this was specified as an "upcoming feature", it must be recognized
-    // as one.
     auto feature = getUpcomingFeature(value);
-    if (enableUpcoming || option.matches(OPT_disable_upcoming_feature)) {
-      if (!feature)
+    if (feature) {
+      // Diagnose upcoming features enabled with -enable-experimental-feature.
+      if (!isUpcomingFlag)
+        Diags.diagnose(SourceLoc(), diag::feature_not_experimental, value,
+                       enableFeature);
+    } else {
+      // If -enable-upcoming-feature was used and an upcoming feature was not
+      // found, diagnose and continue.
+      if (isUpcomingFlag) {
+        Diags.diagnose(SourceLoc(), diag::unrecognized_feature, value,
+                       /*upcoming=*/true);
         continue;
-    }
+      }
 
-    // If it's not recognized as either an upcoming feature or an experimental
-    // feature, skip it.
-    if (!feature) {
+      // If the feature is also not a recognized experimental feature, skip it.
       feature = getExperimentalFeature(value);
-      if (!feature)
+      if (!feature) {
+        Diags.diagnose(SourceLoc(), diag::unrecognized_feature, value,
+                       /*upcoming=*/false);
         continue;
+      }
     }
 
     // Skip features that are already enabled or disabled.
@@ -2369,6 +2366,9 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
 
 static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 DiagnosticEngine &Diags) {
+  // NOTE: This executes at the beginning of parsing the command line and cannot
+  // depend on the results of parsing other options.
+
   using namespace options;
 
   if (Args.hasArg(OPT_verify))
@@ -2492,6 +2492,56 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
   return false;
 }
 
+static void configureDiagnosticEngine(
+    const DiagnosticOptions &Options,
+    std::optional<version::Version> effectiveLanguageVersion,
+    StringRef mainExecutablePath, DiagnosticEngine &Diagnostics) {
+  if (Options.ShowDiagnosticsAfterFatalError) {
+    Diagnostics.setShowDiagnosticsAfterFatalError();
+  }
+  if (Options.SuppressWarnings) {
+    Diagnostics.setSuppressWarnings(true);
+  }
+  if (Options.SuppressRemarks) {
+    Diagnostics.setSuppressRemarks(true);
+  }
+  Diagnostics.setWarningsAsErrorsRules(Options.WarningsAsErrorsRules);
+  Diagnostics.setPrintDiagnosticNamesMode(Options.PrintDiagnosticNames);
+
+  std::string docsPath = Options.DiagnosticDocumentationPath;
+  if (docsPath.empty()) {
+    // Default to a location relative to the compiler.
+    llvm::SmallString<128> docsPathBuffer(mainExecutablePath);
+    llvm::sys::path::remove_filename(docsPathBuffer); // Remove /swift
+    llvm::sys::path::remove_filename(docsPathBuffer); // Remove /bin
+    llvm::sys::path::append(docsPathBuffer, "share", "doc", "swift",
+                            "diagnostics");
+    docsPath = docsPathBuffer.str();
+  }
+  Diagnostics.setDiagnosticDocumentationPath(docsPath);
+
+  if (!Options.LocalizationCode.empty()) {
+    std::string locPath = Options.LocalizationPath;
+    if (locPath.empty()) {
+      llvm::SmallString<128> locPathBuffer(mainExecutablePath);
+      llvm::sys::path::remove_filename(locPathBuffer); // Remove /swift
+      llvm::sys::path::remove_filename(locPathBuffer); // Remove /bin
+      llvm::sys::path::append(locPathBuffer, "share", "swift", "diagnostics");
+      locPath = locPathBuffer.str();
+    }
+    Diagnostics.setLocalization(Options.LocalizationCode, locPath);
+  }
+
+  if (effectiveLanguageVersion)
+    Diagnostics.setLanguageVersion(*effectiveLanguageVersion);
+}
+
+/// Configures the diagnostic engine for the invocation's options.
+void CompilerInvocation::setUpDiagnosticEngine(DiagnosticEngine &diags) {
+  configureDiagnosticEngine(DiagnosticOpts, LangOpts.EffectiveLanguageVersion,
+                            FrontendOpts.MainExecutablePath, diags);
+}
+
 /// Parse -enforce-exclusivity=... options
 void parseExclusivityEnforcementOptions(const llvm::opt::Arg *A,
                                         SILOptions &Opts,
@@ -3742,6 +3792,16 @@ bool CompilerInvocation::parseArgs(
     return true;
   }
 
+  // Parse options that control diagnostic behavior as early as possible, so
+  // that they can influence the behavior of diagnostics emitted during the
+  // rest of parsing.
+  if (ParseDiagnosticArgs(DiagnosticOpts, ParsedArgs, Diags)) {
+    return true;
+  }
+  configureDiagnosticEngine(DiagnosticOpts,
+                            /*effectiveLanguageVersion=*/std::nullopt,
+                            mainExecutablePath, Diags);
+
   ParseAssertionArgs(ParsedArgs);
 
   if (ParseFrontendArgs(FrontendOpts, ParsedArgs, Diags,
@@ -3796,10 +3856,6 @@ bool CompilerInvocation::parseArgs(
     return true;
   }
 
-  if (ParseDiagnosticArgs(DiagnosticOpts, ParsedArgs, Diags)) {
-    return true;
-  }
-
   if (ParseMigratorArgs(MigratorOpts, LangOpts, FrontendOpts,
                         SearchPathOpts.RuntimeResourcePath, ParsedArgs, Diags)) {
     return true;

lib/Frontend/Frontend.cpp

@@ -727,28 +727,11 @@ void CompilerInstance::setUpLLVMArguments() {
 }
 
 void CompilerInstance::setUpDiagnosticOptions() {
-  if (Invocation.getDiagnosticOptions().ShowDiagnosticsAfterFatalError) {
-    Diagnostics.setShowDiagnosticsAfterFatalError();
-  }
-  if (Invocation.getDiagnosticOptions().SuppressWarnings) {
-    Diagnostics.setSuppressWarnings(true);
-  }
-  if (Invocation.getDiagnosticOptions().SuppressRemarks) {
-    Diagnostics.setSuppressRemarks(true);
-  }
-  Diagnostics.setWarningsAsErrorsRules(
-      Invocation.getDiagnosticOptions().WarningsAsErrorsRules);
-  Diagnostics.setPrintDiagnosticNamesMode(
-      Invocation.getDiagnosticOptions().PrintDiagnosticNames);
-  Diagnostics.setDiagnosticDocumentationPath(
-      Invocation.getDiagnosticOptions().DiagnosticDocumentationPath);
-  Diagnostics.setLanguageVersion(
-      Invocation.getLangOptions().EffectiveLanguageVersion);
-  if (!Invocation.getDiagnosticOptions().LocalizationCode.empty()) {
-    Diagnostics.setLocalization(
-        Invocation.getDiagnosticOptions().LocalizationCode,
-        Invocation.getDiagnosticOptions().LocalizationPath);
-  }
+  // As a side-effect, argument parsing will have already partially configured
+  // the diagnostic engine. However, it needs to be reconfigured here in case
+  // there is any new configuration that should effect diagnostics for the rest
+  // of the compile.
+  Invocation.setUpDiagnosticEngine(Diagnostics);
 }
 
 // The ordering of ModuleLoaders is important!

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -2248,10 +2248,6 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
     subInstance.addDiagnosticConsumer(&noopConsumer);
   }
 
-  // Eagerly suppress warnings if necessary, before parsing arguments.
-  if (subInvocation.getDiagnosticOptions().SuppressWarnings)
-    subInstance.getDiags().setSuppressWarnings(true);
-
   SwiftInterfaceInfo interfaceInfo;
   // Extract compiler arguments from the interface file and use them to configure
   // the compiler invocation.

test/Frontend/strict_features.swift

@@ -0,0 +1,18 @@
+// RUN: %target-swift-frontend -typecheck -enable-upcoming-feature UnknownFeature %s 2>&1 | %FileCheck %s --check-prefix=CHECK-NO-DIAGS --allow-empty
+
+// With -Wwarning StrictLanguageFeatures, emit extra diagnostics for
+// misspecified features.
+// RUN: %target-swift-frontend -typecheck -Wwarning StrictLanguageFeatures -enable-upcoming-feature UnknownFeature %s 2>&1 | %FileCheck %s --check-prefix=CHECK-UNKNOWN-UPCOMING
+// RUN: %target-swift-frontend -typecheck -Wwarning StrictLanguageFeatures -enable-experimental-feature UnknownFeature %s 2>&1 | %FileCheck %s --check-prefix=CHECK-UNKNOWN-EXPERIMENTAL
+// RUN: %target-swift-frontend -typecheck -Wwarning StrictLanguageFeatures -swift-version 5 -enable-experimental-feature ConciseMagicFile %s 2>&1 | %FileCheck %s --check-prefix=CHECK-NOT-EXPERIMENTAL-ENABLE
+// RUN: %target-swift-frontend -typecheck -Wwarning StrictLanguageFeatures -swift-version 5 -enable-experimental-feature ConciseMagicFile -disable-experimental-feature ConciseMagicFile %s 2>&1 | %FileCheck %s --check-prefix=CHECK-NOT-EXPERIMENTAL-DISABLE
+
+// REQUIRES: swift_feature_ConciseMagicFile
+
+// CHECK-NO-DIAGS-NOT: warning:
+// CHECK-NO-DIAGS-NOT: error:
+
+// CHECK-UNKNOWN-UPCOMING: warning: 'UnknownFeature' is not a recognized upcoming feature
+// CHECK-UNKNOWN-EXPERIMENTAL: warning: 'UnknownFeature' is not a recognized experimental feature
+// CHECK-NOT-EXPERIMENTAL-ENABLE: warning: 'ConciseMagicFile' is not an experimental feature, use -enable-upcoming-feature instead
+// CHECK-NOT-EXPERIMENTAL-DISABLE: warning: 'ConciseMagicFile' is not an experimental feature, use -disable-upcoming-feature instead

test/Frontend/upcoming_feature.swift

@@ -37,7 +37,6 @@
 // RUN: %target-swift-frontend -typecheck -disable-experimental-feature ConciseMagicFile -swift-version 6 %s 2>&1 | %FileCheck %s
 
 // REQUIRES: swift_feature_ConciseMagicFile
-// REQUIRES: !swift_feature_UnknownFeature
 
 // CHECK: warning: upcoming feature 'ConciseMagicFile' is already enabled as of Swift version 6
 

test/Misc/verify-swift-feature-testing.test-sh

@@ -16,6 +16,7 @@ EXCEPTIONAL_FILES = [
     pathlib.Path("test/Frontend/experimental-features-no-asserts.swift"),
     # Tests for UnknownFeature not existing
     pathlib.Path("test/Frontend/upcoming_feature.swift"),
+    pathlib.Path("test/Frontend/strict_features.swift"),
     # Tests for ModuleInterfaceExportAs being ignored
     pathlib.Path("test/ModuleInterface/swift-export-as.swift"),
     # Uses the pseudo-feature AvailabilityMacro=

userdocs/diagnostic_groups/StrictLanguageFeatures.md

@@ -0,0 +1,8 @@
+# Strict language feature enablement
+
+By default, if an unrecognized feature name is specified with the
+`-enable-upcoming-feature` or `-enable-experimental-feature` flags, the compiler
+will ignore it without emitting a diagnostic since some projects must be
+simultaneously compatible with multiple versions of the language and toolchain.
+However, this warning group can be enabled to opt-in to detailed diagnostics
+about misspecified features.