Skip to content

Frontend: Implement optional parsing diagnostics for enabled language features #78642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/swift/AST/DiagnosticGroups.def
Original file line number Diff line number Diff line change
Expand Up @@ -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"
14 changes: 12 additions & 2 deletions include/swift/AST/DiagnosticsFrontend.def
Original file line number Diff line number Diff line change
Expand Up @@ -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', "
Expand Down
10 changes: 9 additions & 1 deletion include/swift/Frontend/Frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
110 changes: 83 additions & 27 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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.
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
27 changes: 5 additions & 22 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
4 changes: 0 additions & 4 deletions lib/Frontend/ModuleInterfaceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 18 additions & 0 deletions test/Frontend/strict_features.swift
Original file line number Diff line number Diff line change
@@ -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
1 change: 0 additions & 1 deletion test/Frontend/upcoming_feature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions test/Misc/verify-swift-feature-testing.test-sh
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
8 changes: 8 additions & 0 deletions userdocs/diagnostic_groups/StrictLanguageFeatures.md
Original file line number Diff line number Diff line change
@@ -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.