[swift-settings] Only allow for a setting to be passed exactly once to #SwiftSettings.

This responds to some feedback on the forums. Most importantly this allows for
us to use variadic generics in the the type system to document whether we allow
for "appending" behavior or not. Previously, for some options we would take the
last behavior (and theoretically) for others would have silently had appending
behavior. This just makes the behavior simple and more explicit.

Author: gottesmm

include/swift/AST/DiagnosticsSema.def

@@ -8399,5 +8399,10 @@ ERROR(swift_settings_invalid_setting, none,
 ERROR(swift_settings_must_be_top_level, none,
       "#SwiftSettings can only be invoked as a top level declaration", ())
 
+ERROR(swift_settings_duplicate_setting, none,
+      "duplicate setting passed to #SwiftSettings", ())
+NOTE(swift_settings_duplicate_setting_original_loc, none,
+     "setting originally passed here", ())
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

lib/AST/Module.cpp

@@ -61,6 +61,7 @@
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/TinyPtrVector.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -4191,20 +4192,41 @@ version::Version ModuleDecl::getLanguageVersionBuiltWith() const {
 //                            MARK: SwiftSettings
 //===----------------------------------------------------------------------===//
 
+static llvm::cl::opt<bool> AllowForDuplicateSwiftSettings(
+    "swift-settings-allow-duplicates",
+    llvm::cl::desc("Option that allows for duplicate SwiftSettings. Just for "
+                   "compiler testing"),
+    llvm::cl::Hidden);
+
 namespace {
 
 enum class SwiftSettingKind {
   Unknown = 0,
   DefaultIsolation,
+
+  LastKind = DefaultIsolation,
 };
 
 struct SwiftSettingsWalker : ASTWalker {
   SourceFile &sf;
   ASTContext &ctx;
   SourceFileLangOptions result;
 
+  SmallVector<Expr *, 1> swiftSettingIndexToOriginalExprMap;
+
   SwiftSettingsWalker(SourceFile &sf, ASTContext &ctx)
-      : sf(sf), ctx(ctx), result() {}
+      : sf(sf), ctx(ctx), result() {
+    // NOTE: We do not store a value for Unknown.
+    for (unsigned i : range(unsigned(SwiftSettingKind::LastKind))) {
+      (void)i;
+      swiftSettingIndexToOriginalExprMap.push_back(nullptr);
+    }
+  }
+
+  Expr *&getOriginalSwiftSetting(SwiftSettingKind kind) {
+    assert(kind != SwiftSettingKind::Unknown);
+    return swiftSettingIndexToOriginalExprMap[unsigned(kind) - 1];
+  }
 
   /// Given a specific CallExpr, pattern matches the CallExpr's first argument
   /// to validate it is MainActor.self. Returns CanType() if the CallExpr has
@@ -4263,13 +4285,30 @@ struct SwiftSettingsWalker : ASTWalker {
         continue;
 
       case SwiftSettingKind::DefaultIsolation:
+        auto *&expr =
+            getOriginalSwiftSetting(SwiftSettingKind::DefaultIsolation);
+
+        // If we already have an expr, emit an error and continue.
+        if (!AllowForDuplicateSwiftSettings && expr) {
+          ctx.Diags.diagnose(arg.getStartLoc(),
+                             diag::swift_settings_duplicate_setting);
+          ctx.Diags.diagnose(
+              expr->getLoc(),
+              diag::swift_settings_duplicate_setting_original_loc);
+          foundValidArg = true;
+          continue;
+        }
+
+        // Otherwise, set things up appropriately.
         if (auto actor = patternMatchDefaultIsolationMainActor(callExpr)) {
+          expr = callExpr;
           result.defaultIsolation = actor;
           foundValidArg = true;
           continue;
         }
 
         if (isa<NilLiteralExpr>(callExpr->getArgs()->getExpr(0))) {
+          expr = callExpr;
           result.defaultIsolation = {Type()};
           foundValidArg = true;
           continue;

test/Sema/swiftsettings.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -enable-experimental-feature SwiftSettings -enable-experimental-feature Macros -c -swift-version 6 -disable-availability-checking -verify %s
+// RUN: %target-swift-frontend -enable-experimental-feature SwiftSettings -enable-experimental-feature Macros -c -swift-version 6 -disable-availability-checking -verify -Xllvm -swift-settings-allow-duplicates %s
 
 // REQUIRES: asserts
 // REQUIRES: concurrency
@@ -15,7 +15,6 @@ actor MyActor {}
 #SwiftSettings(2) // expected-error {{Unrecognized setting passed to #SwiftSettings}}
 // expected-error @-1 {{cannot convert value of type 'Int' to expected argument type 'SwiftSetting'}}
 
-// We should for now just take the last one that is specified.
 #SwiftSettings(.defaultIsolation(MainActor.self),
                .defaultIsolation(nil))
 

test/Sema/swiftsettings_duplicates.swift

@@ -0,0 +1,24 @@
+// RUN: %target-swift-frontend -enable-experimental-feature SwiftSettings -enable-experimental-feature Macros -c -swift-version 6 -disable-availability-checking -verify %s
+
+// REQUIRES: asserts
+// REQUIRES: concurrency
+// REQUIRES: swift_feature_Macros
+// REQUIRES: swift_feature_SwiftSettings
+
+// This test specifically tests the behavior of #SwiftSettings when we find
+// multiple instances of the same setting.
+
+actor MyActor {}
+
+#SwiftSettings(.defaultIsolation(MainActor.self), // expected-note 6 {{setting originally passed here}}
+               .defaultIsolation(nil)) // expected-error {{duplicate setting passed to #SwiftSettings}}
+#SwiftSettings(.defaultIsolation(nil)) // expected-error {{duplicate setting passed to #SwiftSettings}}
+#SwiftSettings(.defaultIsolation(MyActor.self)) // expected-error {{duplicate setting passed to #SwiftSettings}}
+#SwiftSettings(.defaultIsolation(1)) // expected-error {{duplicate setting passed to #SwiftSettings}}
+// expected-error @-1 {{cannot convert value of type 'Int' to expected argument type 'any Actor.Type'}}
+#SwiftSettings(2) // expected-error {{Unrecognized setting passed to #SwiftSettings}}
+// expected-error @-1 {{cannot convert value of type 'Int' to expected argument type 'SwiftSetting'}}
+
+#SwiftSettings(.defaultIsolation(MainActor.self), // expected-error {{duplicate setting passed to #SwiftSettings}}
+               .defaultIsolation(nil)) // expected-error {{duplicate setting passed to #SwiftSettings}}
+