Skip to content

[NCGenerics] allow feature-guarded uses of ~Copyable #69821

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
Nov 14, 2023
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
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,9 @@ namespace swift {
void addChildNote(Diagnostic &&D);
void insertChildNote(unsigned beforeIndex, Diagnostic &&D);
};

/// A diagnostic that has no input arguments, so it is trivially-destructable.
using ZeroArgDiagnostic = Diag<>;

/// Describes an in-flight diagnostic, which is currently active
/// within the diagnostic engine and can be augmented within additional
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -961,8 +961,8 @@ ERROR(cannot_suppress_here,none,
ERROR(only_suppress_copyable,none,
"can only suppress 'Copyable'", ())

ERROR(already_suppressed,none,
"duplicate suppression of %0", (Identifier))
ERROR(already_suppressed_copyable,none,
"duplicate suppression of 'Copyable'", ())

//------------------------------------------------------------------------------
// MARK: Pattern parsing diagnostics
Expand Down
33 changes: 26 additions & 7 deletions include/swift/AST/TypeRepr.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "swift/AST/Attr.h"
#include "swift/AST/DeclContext.h"
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/Identifier.h"
#include "swift/AST/Type.h"
Expand Down Expand Up @@ -195,17 +196,35 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
/// A TypeRepr for a type with a syntax error. Can be used both as a
/// top-level TypeRepr and as a part of other TypeRepr.
///
/// The client should make sure to emit a diagnostic at the construction time
/// (in the parser). All uses of this type should be ignored and not
/// re-diagnosed.
/// The client can either emit a detailed diagnostic at the construction time
/// (in the parser) or store a zero-arg diagnostic in this TypeRepr to be
/// emitted after parsing, during type resolution.
///
/// All uses of this type should be ignored and not re-diagnosed.
class ErrorTypeRepr : public TypeRepr {
SourceRange Range;
llvm::Optional<ZeroArgDiagnostic> DelayedDiag;

ErrorTypeRepr(SourceRange Range, llvm::Optional<ZeroArgDiagnostic> Diag)
: TypeRepr(TypeReprKind::Error), Range(Range), DelayedDiag(Diag) {}

public:
ErrorTypeRepr() : TypeRepr(TypeReprKind::Error) {}
ErrorTypeRepr(SourceLoc Loc) : TypeRepr(TypeReprKind::Error), Range(Loc) {}
ErrorTypeRepr(SourceRange Range)
: TypeRepr(TypeReprKind::Error), Range(Range) {}
static ErrorTypeRepr *
create(ASTContext &Context, SourceRange Range,
llvm::Optional<ZeroArgDiagnostic> DelayedDiag = llvm::None) {
assert((!DelayedDiag || Range) && "diagnostic needs a location");
return new (Context) ErrorTypeRepr(Range, DelayedDiag);
}

static ErrorTypeRepr *
create(ASTContext &Context, SourceLoc Loc = SourceLoc(),
llvm::Optional<ZeroArgDiagnostic> DelayedDiag = llvm::None) {
return create(Context, SourceRange(Loc), DelayedDiag);
}

/// If there is a delayed diagnostic stored in this TypeRepr, consumes and
/// emits that diagnostic.
void dischargeDiagnostic(ASTContext &Context);

static bool classof(const TypeRepr *T) {
return T->getKind() == TypeReprKind::Error;
Expand Down
6 changes: 1 addition & 5 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace llvm {

namespace swift {
class IdentTypeRepr;
class ErrorTypeRepr;
class CodeCompletionCallbacks;
class DoneParsingCallback;
class IDEInspectionCallbacksFactory;
Expand Down Expand Up @@ -2035,11 +2036,6 @@ class Parser {
void performIDEInspectionSecondPassImpl(
IDEInspectionDelayedDeclState &info);

/// Returns true if the caller should skip calling `parseType` afterwards.
bool parseLegacyTildeCopyable(SourceLoc *parseTildeCopyable,
ParserStatus &Status,
SourceLoc &TildeCopyableLoc);

//===--------------------------------------------------------------------===//
// ASTGen support.

Expand Down
3 changes: 3 additions & 0 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@

using namespace swift;

static_assert(IsTriviallyDestructible<ZeroArgDiagnostic>::value,
"ZeroArgDiagnostic is meant to be trivially destructable");

namespace {
enum class DiagnosticOptions {
/// No options.
Expand Down
9 changes: 9 additions & 0 deletions lib/AST/TypeRepr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,15 @@ void SILBoxTypeRepr::printImpl(ASTPrinter &Printer,
Printer.printKeyword("sil_box", Opts);
}

void ErrorTypeRepr::dischargeDiagnostic(swift::ASTContext &Context) {
if (!DelayedDiag)
return;

// Consume and emit the diagnostic.
Context.Diags.diagnose(Range.Start, *DelayedDiag).highlight(Range);
DelayedDiag = llvm::None;
}

// See swift/Basic/Statistic.h for declaration: this enables tracing
// TypeReprs, is defined here to avoid too much layering violation / circular
// linkage dependency.
Expand Down
93 changes: 42 additions & 51 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6426,51 +6426,6 @@ static void addMoveOnlyAttrIf(SourceLoc const &parsedTildeCopyable,
attrs.add(new(Context) MoveOnlyAttr(/*IsImplicit=*/true));
}

bool Parser::parseLegacyTildeCopyable(SourceLoc *parseTildeCopyable,
ParserStatus &Status,
SourceLoc &TildeCopyableLoc) {
// Is suppression permitted?
if (parseTildeCopyable) {
// Try to find '~' 'Copyable'
//
// We do this knowing that Copyable is not a real type as of now, so we
// can't rely on parseType.
if (Tok.isTilde()) {
const auto &nextTok = peekToken(); // lookahead
if (isIdentifier(nextTok, Context.Id_Copyable.str())) {
auto tildeLoc = consumeToken();
consumeToken(); // the 'Copyable' token

if (TildeCopyableLoc)
diagnose(tildeLoc, diag::already_suppressed, Context.Id_Copyable);
else
TildeCopyableLoc = tildeLoc;

return true;
} else if (nextTok.is(tok::code_complete)) {
consumeToken(); // consume '~'
Status.setHasCodeCompletionAndIsError();
if (CodeCompletionCallbacks) {
CodeCompletionCallbacks->completeWithoutConstraintType();
}
consumeToken(tok::code_complete);
}

// can't suppress whatever is between '~' and ',' or '{'.
diagnose(Tok, diag::only_suppress_copyable);
consumeToken();
}

} else if (Tok.isTilde()) {
// a suppression isn't allowed here, so emit an error eat the token to
// prevent further parsing errors.
diagnose(Tok, diag::cannot_suppress_here);
consumeToken();
}

return false;
}

/// Parse an inheritance clause.
///
/// \verbatim
Expand Down Expand Up @@ -6546,11 +6501,47 @@ ParserStatus Parser::parseInheritance(
continue;
}

if (!EnabledNoncopyableGenerics) {
if (parseLegacyTildeCopyable(parseTildeCopyable,
Status,
TildeCopyableLoc))
continue;
if (!EnabledNoncopyableGenerics && Tok.isTilde()) {
ErrorTypeRepr *error = nullptr;
if (parseTildeCopyable) {
const auto &nextTok = peekToken(); // lookahead
if (isIdentifier(nextTok, Context.Id_Copyable.str())) {
auto tildeLoc = consumeToken();
consumeToken(); // the 'Copyable' token

if (TildeCopyableLoc)
Inherited.push_back(InheritedEntry(
ErrorTypeRepr::create(Context, tildeLoc,
diag::already_suppressed_copyable)));
else
TildeCopyableLoc = tildeLoc;

continue; // success
}

if (nextTok.is(tok::code_complete)) {
consumeToken(); // consume '~'
Status.setHasCodeCompletionAndIsError();
if (CodeCompletionCallbacks) {
CodeCompletionCallbacks->completeWithoutConstraintType();
}
consumeToken(tok::code_complete);
}

// can't suppress whatever is between '~' and ',' or '{'.
error = ErrorTypeRepr::create(Context, consumeToken(),
diag::only_suppress_copyable);
} else {
// Otherwise, a suppression isn't allowed here unless noncopyable
// generics is enabled, so record a delayed error diagnostic and
// eat the token to prevent further parsing errors.
error = ErrorTypeRepr::create(Context, consumeToken(),
diag::cannot_suppress_here);
}

// Record the error parsing ~Copyable, but continue on to parseType.
if (error)
Inherited.push_back(InheritedEntry(error));
}

auto ParsedTypeResult = parseType();
Expand Down Expand Up @@ -9665,7 +9656,7 @@ Parser::parseDeclSubscript(SourceLoc StaticLoc,

if (ElementTy.isNull()) {
// Always set an element type.
ElementTy = makeParserResult(ElementTy, new (Context) ErrorTypeRepr());
ElementTy = makeParserResult(ElementTy, ErrorTypeRepr::create(Context));
}
}

Expand Down
8 changes: 4 additions & 4 deletions lib/Parse/ParseGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ ParserStatus Parser::parseGenericWhereClause(
ParserResult<TypeRepr> Protocol = parseType();
Status |= Protocol;
if (Protocol.isNull())
Protocol = makeParserResult(new (Context) ErrorTypeRepr(PreviousLoc));
Protocol = makeParserResult(ErrorTypeRepr::create(Context, PreviousLoc));

// Add the requirement.
Requirements.push_back(RequirementRepr::getTypeConstraint(
Expand All @@ -358,7 +358,7 @@ ParserStatus Parser::parseGenericWhereClause(
ParserResult<TypeRepr> SecondType = parseType();
Status |= SecondType;
if (SecondType.isNull())
SecondType = makeParserResult(new (Context) ErrorTypeRepr(PreviousLoc));
SecondType = makeParserResult(ErrorTypeRepr::create(Context, PreviousLoc));

// Add the requirement
if (FirstType.hasCodeCompletion()) {
Expand All @@ -373,7 +373,7 @@ ParserStatus Parser::parseGenericWhereClause(
// completion token in the TypeRepr.
Requirements.push_back(RequirementRepr::getTypeConstraint(
FirstType.get(), EqualLoc,
new (Context) ErrorTypeRepr(SecondType.get()->getLoc()),
ErrorTypeRepr::create(Context, SecondType.get()->getLoc()),
isRequirementExpansion));
} else {
Requirements.push_back(RequirementRepr::getSameType(
Expand All @@ -383,7 +383,7 @@ ParserStatus Parser::parseGenericWhereClause(
} else if (FirstType.hasCodeCompletion()) {
// Recover by adding dummy constraint.
Requirements.push_back(RequirementRepr::getTypeConstraint(
FirstType.get(), PreviousLoc, new (Context) ErrorTypeRepr(PreviousLoc),
FirstType.get(), PreviousLoc, ErrorTypeRepr::create(Context, PreviousLoc),
isRequirementExpansion));
} else {
diagnose(Tok, diag::expected_requirement_delim);
Expand Down
4 changes: 2 additions & 2 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ ParserResult<Pattern> Parser::parseTypedPattern() {
}
}
} else {
Ty = makeParserResult(new (Context) ErrorTypeRepr(PreviousLoc));
Ty = makeParserResult(ErrorTypeRepr::create(Context, PreviousLoc));
}

result = makeParserResult(result,
Expand Down Expand Up @@ -1278,7 +1278,7 @@ parseOptionalPatternTypeAnnotation(ParserResult<Pattern> result) {

TypeRepr *repr = Ty.getPtrOrNull();
if (!repr)
repr = new (Context) ErrorTypeRepr(PreviousLoc);
repr = ErrorTypeRepr::create(Context, PreviousLoc);

return makeParserResult(status, new (Context) TypedPattern(P, repr));
}
Expand Down
27 changes: 15 additions & 12 deletions lib/Parse/ParseType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,6 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
SourceLoc tildeLoc;
if (Tok.isTilde() && !isInSILMode()) {
tildeLoc = consumeToken();
if (!EnabledNoncopyableGenerics)
diagnose(tildeLoc, diag::cannot_suppress_here)
.fixItRemoveChars(tildeLoc, tildeLoc);
}

switch (Tok.getKind()) {
Expand Down Expand Up @@ -232,7 +229,7 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
CodeCompletionCallbacks->completeTypeSimpleBeginning();
}
return makeParserCodeCompletionResult<TypeRepr>(
new (Context) ErrorTypeRepr(consumeToken(tok::code_complete)));
ErrorTypeRepr::create(Context, consumeToken(tok::code_complete)));
case tok::l_square: {
ty = parseTypeCollection();
break;
Expand All @@ -255,7 +252,7 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
diag.fixItInsert(getEndOfPreviousLoc(), " <#type#>");
}
if (Tok.isKeyword() && !Tok.isAtStartOfLine()) {
ty = makeParserErrorResult(new (Context) ErrorTypeRepr(Tok.getLoc()));
ty = makeParserErrorResult(ErrorTypeRepr::create(Context, Tok.getLoc()));
consumeToken();
return ty;
}
Expand Down Expand Up @@ -310,9 +307,15 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
}

// Wrap in an InverseTypeRepr if needed.
if (EnabledNoncopyableGenerics && tildeLoc) {
ty = makeParserResult(ty,
new(Context) InverseTypeRepr(tildeLoc, ty.get()));
if (tildeLoc) {
TypeRepr *repr;
if (EnabledNoncopyableGenerics)
repr = new (Context) InverseTypeRepr(tildeLoc, ty.get());
else
repr =
ErrorTypeRepr::create(Context, tildeLoc, diag::cannot_suppress_here);

ty = makeParserResult(ty, repr);
}

return ty;
Expand Down Expand Up @@ -685,8 +688,8 @@ ParserResult<TypeRepr> Parser::parseDeclResultType(Diag<> MessageID) {
auto diag = diagnose(Tok, diag::extra_rbracket);
diag.fixItInsert(result.get()->getStartLoc(), getTokenText(tok::l_square));
consumeToken();
return makeParserErrorResult(new (Context)
ErrorTypeRepr(getTypeErrorLoc()));
return makeParserErrorResult(ErrorTypeRepr::create(Context,
getTypeErrorLoc()));
}

if (Tok.is(tok::colon)) {
Expand All @@ -702,8 +705,8 @@ ParserResult<TypeRepr> Parser::parseDeclResultType(Diag<> MessageID) {
diag.fixItInsertAfter(secondType.get()->getEndLoc(), getTokenText(tok::r_square));
}
}
return makeParserErrorResult(new (Context)
ErrorTypeRepr(getTypeErrorLoc()));
return makeParserErrorResult(ErrorTypeRepr::create(Context,
getTypeErrorLoc()));
}
}
return result;
Expand Down
6 changes: 3 additions & 3 deletions lib/Sema/PreCheckExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,7 @@ TypeExpr *PreCheckExpression::simplifyTypeExpr(Expr *E) {
Ctx.Diags.diagnose(AE->getArgsExpr()->getLoc(),
diag::expected_type_before_arrow);
auto ArgRange = AE->getArgsExpr()->getSourceRange();
auto ErrRepr = new (Ctx) ErrorTypeRepr(ArgRange);
auto ErrRepr = ErrorTypeRepr::create(Ctx, ArgRange);
ArgsTypeRepr =
TupleTypeRepr::create(Ctx, {ErrRepr}, ArgRange);
}
Expand All @@ -2025,8 +2025,8 @@ TypeExpr *PreCheckExpression::simplifyTypeExpr(Expr *E) {
if (!ResultTypeRepr) {
Ctx.Diags.diagnose(AE->getResultExpr()->getLoc(),
diag::expected_type_after_arrow);
ResultTypeRepr = new (Ctx)
ErrorTypeRepr(AE->getResultExpr()->getSourceRange());
ResultTypeRepr =
ErrorTypeRepr::create(Ctx, AE->getResultExpr()->getSourceRange());
}

auto NewTypeRepr = new (Ctx)
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/TypeCheckType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2431,6 +2431,7 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,

switch (repr->getKind()) {
case TypeReprKind::Error:
cast<ErrorTypeRepr>(repr)->dischargeDiagnostic(getASTContext());
return ErrorType::get(getASTContext());

case TypeReprKind::Attributed:
Expand Down
Loading