Skip to content

[send-non-sendable] Convert some of the concurrency tests that test targeted concurrency to also test complete and complete + sis #68195

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
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
8 changes: 8 additions & 0 deletions include/swift/Basic/DiagnosticOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ class DiagnosticOptions {
/// Path to a directory of diagnostic localization tables.
std::string LocalizationPath = "";

/// A list of prefixes that are appended to expected- that the diagnostic
/// verifier should check for diagnostics.
///
/// For example, if one placed the phrase "NAME", the verifier will check for:
/// expected-$NAME{error,note,warning,remark} as well as the normal expected-
/// prefixes.
std::vector<std::string> AdditionalDiagnosticVerifierPrefixes;

/// Return a hash code of any components from these options that should
/// contribute to a Swift Bridging PCH hash.
llvm::hash_code getPCHHashComponents() const {
Expand Down
7 changes: 5 additions & 2 deletions include/swift/Frontend/DiagnosticVerifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,15 @@ class DiagnosticVerifier : public DiagnosticConsumer {
SmallVector<unsigned, 4> AdditionalBufferIDs;
bool AutoApplyFixes;
bool IgnoreUnknown;
ArrayRef<std::string> AdditionalExpectedPrefixes;

public:
explicit DiagnosticVerifier(SourceManager &SM, ArrayRef<unsigned> BufferIDs,
bool AutoApplyFixes, bool IgnoreUnknown)
bool AutoApplyFixes, bool IgnoreUnknown,
ArrayRef<std::string> AdditionalExpectedPrefixes)
: SM(SM), BufferIDs(BufferIDs), AutoApplyFixes(AutoApplyFixes),
IgnoreUnknown(IgnoreUnknown) {}
IgnoreUnknown(IgnoreUnknown),
AdditionalExpectedPrefixes(AdditionalExpectedPrefixes) {}

void appendAdditionalBufferID(unsigned bufferID) {
AdditionalBufferIDs.push_back(bufferID);
Expand Down
2 changes: 2 additions & 0 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ def verify : Flag<["-"], "verify">,
"annotations">;
def verify_additional_file : Separate<["-"], "verify-additional-file">,
HelpText<"Verify diagnostics in this file in addition to source files">;
def verify_additional_prefix : Separate<["-"], "verify-additional-prefix">,
HelpText<"Check for diagnostics with the prefix expected-<PREFIX> as well as expected-">;
def verify_apply_fixes : Flag<["-"], "verify-apply-fixes">,
HelpText<"Like -verify, but updates the original source file">;
def verify_ignore_unknown: Flag<["-"], "verify-ignore-unknown">,
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,8 @@ static bool ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,

for (Arg *A : Args.filtered(OPT_verify_additional_file))
Opts.AdditionalVerifierFiles.push_back(A->getValue());
for (Arg *A : Args.filtered(OPT_verify_additional_prefix))
Opts.AdditionalDiagnosticVerifierPrefixes.push_back(A->getValue());

Opts.UseColor |=
Args.hasFlag(OPT_color_diagnostics,
Expand Down
126 changes: 109 additions & 17 deletions lib/Frontend/DiagnosticVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,81 @@

using namespace swift;

namespace {

struct ExpectedCheckMatchStartParser {
StringRef MatchStart;
const char *ClassificationStartLoc = nullptr;
std::optional<DiagnosticKind> ExpectedClassification;

ExpectedCheckMatchStartParser(StringRef MatchStart)
: MatchStart(MatchStart) {}

bool tryParseClassification() {
if (MatchStart.startswith("note")) {
ClassificationStartLoc = MatchStart.data();
ExpectedClassification = DiagnosticKind::Note;
MatchStart = MatchStart.substr(strlen("note"));
return true;
}

if (MatchStart.startswith("warning")) {
ClassificationStartLoc = MatchStart.data();
ExpectedClassification = DiagnosticKind::Warning;
MatchStart = MatchStart.substr(strlen("warning"));
return true;
}

if (MatchStart.startswith("error")) {
ClassificationStartLoc = MatchStart.data();
ExpectedClassification = DiagnosticKind::Error;
MatchStart = MatchStart.substr(strlen("error"));
return true;
}

if (MatchStart.startswith("remark")) {
ClassificationStartLoc = MatchStart.data();
ExpectedClassification = DiagnosticKind::Remark;
MatchStart = MatchStart.substr(strlen("remark"));
return true;
}

return false;
}

bool parse(ArrayRef<std::string> prefixes) {
// First try to parse as if we did not have a prefix. We always parse at
// least expected-*.
if (tryParseClassification())
return true;

// Otherwise, walk our prefixes until we find one that matches and attempt
// to check for a note, warning, error, or remark.
//
// TODO: We could make this more flexible, but this should work in the
// short term.
for (auto &p : prefixes) {
if (MatchStart.starts_with(p)) {
MatchStart = MatchStart.substr(p.size());
return tryParseClassification();
}
}

return false;
}
};

} // anonymous namespace

namespace swift {

struct ExpectedFixIt {
const char *StartLoc, *EndLoc; // The loc of the {{ and }}'s.
LineColumnRange Range;

std::string Text;
};

} // end namespace swift

const LineColumnRange &
Expand Down Expand Up @@ -460,6 +528,28 @@ static llvm::Optional<LineColumnRange> parseExpectedFixItRange(
return Range;
}

/// Before we do anything, check if any of our prefixes are prefixes of later
/// prefixes. In such a case, we will never actually pattern match the later
/// prefix. In such a case, crash with a nice error message.
static void validatePrefixList(ArrayRef<std::string> prefixes) {
// Work backwards through the prefix list.
while (!prefixes.empty()) {
auto target = StringRef(prefixes.front());
prefixes = prefixes.drop_front();

for (auto &p : prefixes) {
if (StringRef(p).starts_with(target)) {
llvm::errs() << "Error! Found a verifier diagnostic additional prefix "
"that is a prefix of a later prefix. The later prefix "
"will never be pattern matched!\n"
<< "First Prefix: " << target << '\n'
<< "Second Prefix: " << p << '\n';
llvm::report_fatal_error("Standard compiler error!\n");
}
}
}
}

/// After the file has been processed, check to see if we got all of
/// the expected diagnostics and check to see if there were any unexpected
/// ones.
Expand All @@ -486,6 +576,10 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
Errors.push_back(diag);
};

// Validate that earlier prefixes are not prefixes of alter
// prefixes... otherwise, we will never pattern match the later prefix.
validatePrefixList(AdditionalExpectedPrefixes);

// Scan the memory buffer looking for expected-note/warning/error.
for (size_t Match = InputFile.find("expected-");
Match != StringRef::npos; Match = InputFile.find("expected-", Match+1)) {
Expand All @@ -494,23 +588,21 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {
StringRef MatchStart = InputFile.substr(Match);
const char *DiagnosticLoc = MatchStart.data();
MatchStart = MatchStart.substr(strlen("expected-"));
const char *ClassificationStartLoc = MatchStart.data();

DiagnosticKind ExpectedClassification;
if (MatchStart.startswith("note")) {
ExpectedClassification = DiagnosticKind::Note;
MatchStart = MatchStart.substr(strlen("note"));
} else if (MatchStart.startswith("warning")) {
ExpectedClassification = DiagnosticKind::Warning;
MatchStart = MatchStart.substr(strlen("warning"));
} else if (MatchStart.startswith("error")) {
ExpectedClassification = DiagnosticKind::Error;
MatchStart = MatchStart.substr(strlen("error"));
} else if (MatchStart.startswith("remark")) {
ExpectedClassification = DiagnosticKind::Remark;
MatchStart = MatchStart.substr(strlen("remark"));
} else
continue;
const char *ClassificationStartLoc = nullptr;
std::optional<DiagnosticKind> ExpectedClassification;
{
ExpectedCheckMatchStartParser parser(MatchStart);
// If we fail to parse... continue.
if (!parser.parse(AdditionalExpectedPrefixes)) {
continue;
}
MatchStart = parser.MatchStart;
ClassificationStartLoc = parser.ClassificationStartLoc;
ExpectedClassification = parser.ExpectedClassification;
}
assert(ClassificationStartLoc);
assert(bool(ExpectedClassification));

// Skip any whitespace before the {{.
MatchStart = MatchStart.substr(MatchStart.find_first_not_of(" \t"));
Expand All @@ -525,7 +617,7 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) {

ExpectedDiagnosticInfo Expected(DiagnosticLoc, ClassificationStartLoc,
/*ClassificationEndLoc=*/MatchStart.data(),
ExpectedClassification);
*ExpectedClassification);
int LineOffset = 0;

if (TextStartIdx > 0 && MatchStart[0] == '@') {
Expand Down
3 changes: 2 additions & 1 deletion lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,8 @@ bool CompilerInstance::setupDiagnosticVerifierIfNeeded() {
DiagVerifier = std::make_unique<DiagnosticVerifier>(
SourceMgr, InputSourceCodeBufferIDs,
diagOpts.VerifyMode == DiagnosticOptions::VerifyAndApplyFixes,
diagOpts.VerifyIgnoreUnknown);
diagOpts.VerifyIgnoreUnknown,
diagOpts.AdditionalDiagnosticVerifierPrefixes);
for (const auto &filename : diagOpts.AdditionalVerifierFiles) {
auto result = getFileSystem().getBufferForFile(filename);
if (!result) {
Expand Down
6 changes: 5 additions & 1 deletion test/Concurrency/LLDBDebuggerFunctionActorExtension.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking -debugger-support
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking -debugger-support %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-experimental-feature SendNonSendable

// REQUIRES: concurrency

// This test simulates LLDB's expression evaluator making an otherwise illegal
Expand Down
20 changes: 11 additions & 9 deletions test/Concurrency/actor_call_implicitly_async.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking -warn-concurrency -parse-as-library
// RUN: %target-swift-frontend -disable-availability-checking -warn-concurrency -parse-as-library %s -emit-sil -o /dev/null -verify -verify-additional-prefix complete-
// RUN: %target-swift-frontend -disable-availability-checking -warn-concurrency -parse-as-library %s -emit-sil -o /dev/null -verify -enable-experimental-feature SendNonSendable

// REQUIRES: concurrency


Expand Down Expand Up @@ -289,8 +291,8 @@ func blender(_ peeler : () -> Void) {


await wisk({})
// expected-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
// expected-complete-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
await wisk(1)
await (peelBanana)()
await (((((peelBanana)))))()
Expand All @@ -300,15 +302,15 @@ func blender(_ peeler : () -> Void) {
// expected-warning@-1 2{{converting function value of type '@BananaActor () -> ()' to '() -> Void' loses global actor 'BananaActor'}}

await wisk(peelBanana)
// expected-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
// expected-complete-warning@-1{{passing argument of non-sendable type '() -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}

await wisk(wisk)
// expected-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
// expected-complete-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
await (((wisk)))(((wisk)))
// expected-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
// expected-complete-warning@-1{{passing argument of non-sendable type '(Any) -> ()' into global actor 'BananaActor'-isolated context may introduce data races}}
// expected-complete-note@-2{{a function type must be marked '@Sendable' to conform to 'Sendable'}}

await {wisk}()(1)

Expand Down
4 changes: 4 additions & 0 deletions test/Concurrency/actor_defer.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s
// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s -strict-concurrency=targeted
// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s -strict-concurrency=complete
// RUN: %target-swift-frontend -parse-as-library -emit-sil -DNEGATIVES -verify %s -strict-concurrency=complete -enable-experimental-feature SendNonSendable

// RUN: %target-swift-frontend -parse-as-library -emit-sil -enable-actor-data-race-checks -o - %s | %FileCheck %s

// REQUIRES: concurrency
Expand Down
6 changes: 5 additions & 1 deletion test/Concurrency/actor_derived_conformances.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-experimental-feature SendNonSendable

// REQUIRES: concurrency

actor A1: Comparable {}
Expand Down
6 changes: 5 additions & 1 deletion test/Concurrency/actor_existentials.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=targeted
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -enable-experimental-feature SendNonSendable

// REQUIRES: concurrency

protocol P: Actor {
Expand Down
Loading