Skip to content

[ASTPrinter] Fix duplicate «mutating» modifiers when printing file interface #30011

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 1 commit into from
Mar 18, 2020
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
55 changes: 29 additions & 26 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
bool shouldPrintPattern(const Pattern *P);
void printPatternType(const Pattern *P);
void printAccessors(const AbstractStorageDecl *ASD);
void printMutatingModifiersIfNeeded(const AccessorDecl *accessor);
void printMutabilityModifiersIfNeeded(const FuncDecl *FD);
void printMembersOfDecl(Decl * NTD, bool needComma = false,
bool openBracket = true, bool closeBracket = true);
void printMembers(ArrayRef<Decl *> members, bool needComma = false,
Expand Down Expand Up @@ -990,9 +990,6 @@ void PrintAST::printAttributes(const Decl *D) {
#define CONTEXTUAL_SIMPLE_DECL_ATTR(X, Class, Y, Z) EXCLUDE_ATTR(Class)
#define CONTEXTUAL_DECL_ATTR_ALIAS(X, Class) EXCLUDE_ATTR(Class)
#include "swift/AST/Attr.def"
} else if (isa<FuncDecl>(D)) {
Options.ExcludeAttrList.push_back(DAK_Mutating);
Options.ExcludeAttrList.push_back(DAK_NonMutating);
}

// If the declaration is implicitly @objc, print the attribute now.
Expand All @@ -1008,19 +1005,23 @@ void PrintAST::printAttributes(const Decl *D) {
// clients, or if it inherits superclass convenience initializers,
// then print those attributes specially.
if (auto CD = dyn_cast<ClassDecl>(D)) {
if (Options.PrintImplicitAttrs) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benlangmuir This is all nested inside if (Options.PrintImplicitAttrs) up above, so I just eliminated the unnecessary scope.

if (CD->inheritsSuperclassInitializers()) {
Printer.printAttrName("@_inheritsConvenienceInitializers");
Printer << " ";
}
if (CD->hasMissingDesignatedInitializers()) {
Printer.printAttrName("@_hasMissingDesignatedInitializers");
Printer << " ";
}
if (CD->inheritsSuperclassInitializers()) {
Printer.printAttrName("@_inheritsConvenienceInitializers");
Printer << " ";
}
if (CD->hasMissingDesignatedInitializers()) {
Printer.printAttrName("@_hasMissingDesignatedInitializers");
Printer << " ";
}
}
}

// We will handle 'mutating' and 'nonmutating' separately.
if (isa<FuncDecl>(D)) {
Options.ExcludeAttrList.push_back(DAK_Mutating);
Options.ExcludeAttrList.push_back(DAK_NonMutating);
}

D->getAttrs().print(Printer, Options, D);

// Print the implicit 'final' attribute.
Expand Down Expand Up @@ -1758,12 +1759,15 @@ void PrintAST::printBodyIfNecessary(const AbstractFunctionDecl *decl) {
printBraceStmt(decl->getBody(), /*newlineIfEmpty*/!isa<AccessorDecl>(decl));
}

void PrintAST::printMutatingModifiersIfNeeded(const AccessorDecl *accessor) {
if (accessor->isAssumedNonMutating() && accessor->isMutating() &&
!Options.excludeAttrKind(DAK_Mutating)) {
Printer.printKeyword("mutating", Options, " ");
} else if (accessor->isExplicitNonMutating() &&
!Options.excludeAttrKind(DAK_NonMutating)) {
void PrintAST::printMutabilityModifiersIfNeeded(const FuncDecl *FD) {
const auto *AD = dyn_cast<AccessorDecl>(FD);

if (FD->isMutating()) {
if (AD == nullptr || AD->isAssumedNonMutating())
if (!Options.excludeAttrKind(DAK_Mutating))
Printer.printKeyword("mutating", Options, " ");
} else if (AD && AD->isExplicitNonMutating() &&
!Options.excludeAttrKind(DAK_Mutating)) {
Printer.printKeyword("nonmutating", Options, " ");
}
}
Expand Down Expand Up @@ -1881,7 +1885,7 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
return true;
if (!PrintAccessorBody) {
Printer << " ";
printMutatingModifiersIfNeeded(Accessor);
printMutabilityModifiersIfNeeded(Accessor);
Printer.printKeyword(getAccessorLabel(Accessor->getAccessorKind()), Options);
} else {
{
Expand Down Expand Up @@ -2777,10 +2781,9 @@ bool PrintAST::printASTNodes(const ArrayRef<ASTNode> &Elements,
void PrintAST::visitAccessorDecl(AccessorDecl *decl) {
printDocumentationComment(decl);
printAttributes(decl);
// Explicitly print 'mutating' and 'nonmutating' if needed.
printMutabilityModifiersIfNeeded(decl);

// Explicitly print 'mutating' and 'nonmutating' before getters and setters
// for which that is true.
printMutatingModifiersIfNeeded(decl);
switch (auto kind = decl->getAccessorKind()) {
case AccessorKind::Get:
case AccessorKind::Address:
Expand Down Expand Up @@ -2837,9 +2840,9 @@ void PrintAST::visitFuncDecl(FuncDecl *decl) {
if (!Options.SkipIntroducerKeywords) {
if (decl->isStatic() && Options.PrintStaticKeyword)
printStaticKeyword(decl->getCorrectStaticSpelling());
if (decl->isMutating() && !Options.excludeAttrKind(DAK_Mutating)) {
Printer.printKeyword("mutating", Options, " ");
} else if (decl->isConsuming() && !decl->getAttrs().hasAttribute<ConsumingAttr>()) {

printMutabilityModifiersIfNeeded(decl);
if (decl->isConsuming() && !decl->getAttrs().hasAttribute<ConsumingAttr>()) {
Printer.printKeyword("__consuming", Options, " ");
}
Printer << tok::kw_func << " ";
Expand Down
5 changes: 5 additions & 0 deletions test/IDE/print_source_file_interface_2.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,10 @@ extension MyClass {
subscript(i: Int) -> Int { return 0 }
}

/// Don't print `mutating` twice.
struct MyStruct {
mutating func foo() {}
}

// RUN: %target-swift-ide-test -print-swift-file-interface -source-filename %s > %t.out
// RUN: diff -u %s.result %t.out
6 changes: 6 additions & 0 deletions test/IDE/print_source_file_interface_2.swift.result
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,9 @@ extension MyClass {
/// and a nice subscript.
internal subscript(i: Int) -> Int { get }
}

/// Don't print `mutating` twice.
internal struct MyStruct {

internal mutating func foo()
}