Skip to content

ASTPrinter: remove ASTPrinter's dependency on sema. NFC #10985

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.

Sign up for GitHub

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
Jul 15, 2017
Merged

ASTPrinter: remove ASTPrinter's dependency on sema. NFC #10985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 3 deletions include/swift/AST/ASTPrinter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,6 @@ class ExtraIndentStreamPrinter : public StreamPrinter {
}
};

bool shouldPrint(const Decl *D, PrintOptions &Options);
bool shouldPrintPattern(const Pattern *P, PrintOptions &Options);

void printContext(raw_ostream &os, DeclContext *dc);

bool printRequirementStub(ValueDecl *Requirement, DeclContext *Adopter,
Expand Down
58 changes: 19 additions & 39 deletions include/swift/AST/PrintOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace swift {
class GenericEnvironment;
class CanType;
class Decl;
class Pattern;
class ValueDecl;
class ExtensionDecl;
class NominalTypeDecl;
Expand All @@ -49,32 +50,6 @@ struct TypeTransformContext {
bool isPrintingSynthesizedExtension() const;
};

typedef std::pair<ExtensionDecl*, bool> ExtensionAndIsSynthesized;
typedef llvm::function_ref<void(ArrayRef<ExtensionAndIsSynthesized>)>
ExtensionGroupOperation;

class SynthesizedExtensionAnalyzer {
struct Implementation;
Implementation &Impl;
public:
SynthesizedExtensionAnalyzer(NominalTypeDecl *Target,
PrintOptions Options,
bool IncludeUnconditional = true);
~SynthesizedExtensionAnalyzer();

enum class MergeGroupKind : char {
All,
MergeableWithTypeDef,
UnmergeableWithTypeDef,
};

void forEachExtensionMergeGroup(MergeGroupKind Kind,
ExtensionGroupOperation Fn);
bool isInSynthesizedExtension(const ValueDecl *VD);
bool shouldPrintRequirement(ExtensionDecl *ED, StringRef Req);
bool hasMergeGroup(MergeGroupKind Kind);
};

class BracketOptions {
Decl* Target;
bool OpenExtension;
Expand Down Expand Up @@ -131,6 +106,12 @@ class AnyAttrKind {
bool operator!=(AnyAttrKind K) const { return !(*this == K); }
};

struct ShouldPrintChecker {
virtual bool shouldPrint(const Decl *D, PrintOptions &Options);
bool shouldPrint(const Pattern *P, PrintOptions &Options);
virtual ~ShouldPrintChecker() = default;
};

/// Options for printing AST nodes.
///
/// A default-constructed PrintOptions is suitable for printing to users;
Expand Down Expand Up @@ -294,6 +275,9 @@ struct PrintOptions {
/// Whether to print the extensions from conforming protocols.
bool PrintExtensionFromConformingProtocols = false;

std::shared_ptr<ShouldPrintChecker> CurrentPrintabilityChecker =
Copy link
Contributor

Choose a reason for hiding this comment

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

Small note: it seems a little funny to make this a shared_ptr rather than just an optional non-owning pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like Optional<ShouldPrintChecker*>? does it mean we don't release the original one when when resetting the pointer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just ShouldPrintChecker * is fine—if it's nullptr, you get the default behavior. Whenever someone wants to customize it, they can stack-allocate their checker and pass in the address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stack allocation is usually insufficient for setting the checker because we set in the factory method of PrintOptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

:-/ All right. std::shared_ptr just seems very heavyweight to me, but I guess PrintOptions don't get copied around a lot and customizing this behavior doesn't happen very often anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for talking through it with me!

std::make_shared<ShouldPrintChecker>();

enum class ArgAndParamPrintingMode {
ArgumentOnly,
MatchSource,
Expand Down Expand Up @@ -423,6 +407,7 @@ struct PrintOptions {
return result;
}

static PrintOptions printModuleInterface();
static PrintOptions printTypeInterface(Type T);

void setBaseType(Type T);
Expand All @@ -431,6 +416,13 @@ struct PrintOptions {

void clearSynthesizedExtension();

bool shouldPrint(const Decl* D) {
return CurrentPrintabilityChecker->shouldPrint(D, *this);
}
bool shouldPrint(const Pattern* P) {
return CurrentPrintabilityChecker->shouldPrint(P, *this);
}

/// Retrieve the print options that are suitable to print the testable interface.
static PrintOptions printTestableInterface() {
PrintOptions result = printInterface();
Expand All @@ -449,19 +441,7 @@ struct PrintOptions {

/// Retrieve the set of options suitable for interface generation for
/// documentation purposes.
static PrintOptions printDocInterface() {
PrintOptions result = PrintOptions::printInterface();
result.PrintAccessibility = false;
result.SkipUnavailable = false;
result.ExcludeAttrList.push_back(DAK_Available);
result.ArgAndParamPrinting =
PrintOptions::ArgAndParamPrintingMode::BothAlways;
result.PrintDocumentationComments = false;
result.PrintRegularClangComments = false;
result.PrintAccessibility = false;
result.PrintFunctionRepresentationAttrs = false;
return result;
}
static PrintOptions printDocInterface();

/// Retrieve the set of options suitable for printing SIL functions.
static PrintOptions printSIL() {
Expand Down
32 changes: 32 additions & 0 deletions include/swift/Sema/IDETypeChecking.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ namespace swift {
class LazyResolver;
class ExtensionDecl;
class ProtocolDecl;
class Type;
class DeclContext;
class ConcreteDeclRef;
class ValueDecl;
class DeclName;

/// \brief Typecheck a declaration parsed during code completion.
///
Expand Down Expand Up @@ -131,6 +136,33 @@ namespace swift {

/// Creates a lazy type resolver for use in lookups.
OwnedResolver createLazyResolver(ASTContext &Ctx);

typedef std::pair<ExtensionDecl*, bool> ExtensionAndIsSynthesized;

typedef llvm::function_ref<void(ArrayRef<ExtensionAndIsSynthesized>)>
ExtensionGroupOperation;

class SynthesizedExtensionAnalyzer {
struct Implementation;
Implementation &Impl;
public:
SynthesizedExtensionAnalyzer(NominalTypeDecl *Target,
PrintOptions Options,
bool IncludeUnconditional = true);
~SynthesizedExtensionAnalyzer();

enum class MergeGroupKind : char {
All,
MergeableWithTypeDef,
UnmergeableWithTypeDef,
};

void forEachExtensionMergeGroup(MergeGroupKind Kind,
ExtensionGroupOperation Fn);
bool isInSynthesizedExtension(const ValueDecl *VD);
bool shouldPrintRequirement(ExtensionDecl *ED, StringRef Req);
bool hasMergeGroup(MergeGroupKind Kind);
};
}

#endif
71 changes: 21 additions & 50 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "swift/Basic/StringExtras.h"
#include "swift/Config.h"
#include "swift/Parse/Lexer.h"
#include "swift/Sema/IDETypeChecking.h"
#include "swift/Strings.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
Expand All @@ -52,17 +51,6 @@

using namespace swift;

PrintOptions PrintOptions::printTypeInterface(Type T) {
PrintOptions result = printInterface();
result.PrintExtensionFromConformingProtocols = true;
result.TransformContext = TypeTransformContext(T);
result.printExtensionContentAsMembers = [T](const ExtensionDecl *ED) {
return isExtensionApplied(*T->getNominalOrBoundGenericNominal()->
getDeclContext(), T, ED);
};
return result;
}

void PrintOptions::setBaseType(Type T) {
TransformContext = TypeTransformContext(T);
}
Expand Down Expand Up @@ -830,17 +818,17 @@ static const unsigned ErrorDepth = ~0U;
/// A helper function to return the depth of a type.
static unsigned getDepthOfType(Type ty) {
unsigned depth = ErrorDepth;

auto combineDepth = [&depth](unsigned newDepth) -> bool {
// If there is no current depth (depth == ErrorDepth), then assign to
// newDepth; otherwise, choose the deeper of the current and new depth.

// Since ErrorDepth == ~0U, ErrorDepth + 1 == 0, which is smaller than any
// valid depth + 1.
depth = std::max(depth+1U, newDepth+1U) - 1U;
return false;
};

ty.findIf([combineDepth](Type t) -> bool {
if (auto paramTy = t->getAs<GenericTypeParamType>())
return combineDepth(paramTy->getDepth());
Expand All @@ -856,7 +844,7 @@ static unsigned getDepthOfType(Type ty) {

return false;
});

return depth;
}

Expand Down Expand Up @@ -1222,16 +1210,8 @@ void PrintAST::printRequirement(const Requirement &req) {
printType(req.getSecondType());
}

bool swift::shouldPrintPattern(const Pattern *P, PrintOptions &Options) {
bool ShouldPrint = false;
P->forEachVariable([&](VarDecl *VD) {
ShouldPrint |= shouldPrint(VD, Options);
});
return ShouldPrint;
}

bool PrintAST::shouldPrintPattern(const Pattern *P) {
return swift::shouldPrintPattern(P, Options);
return Options.shouldPrint(P);
}

void PrintAST::printPatternType(const Pattern *P) {
Expand All @@ -1241,24 +1221,15 @@ void PrintAST::printPatternType(const Pattern *P) {
}
}

static bool shouldPrintAsFavorable(const Decl *D, PrintOptions &Options) {
if (!Options.TransformContext || !D->getDeclContext()->isExtensionContext() ||
!Options.TransformContext->isPrintingSynthesizedExtension())
return true;
NominalTypeDecl *Target = Options.TransformContext->getNominal();
Type BaseTy = Target->getDeclaredTypeInContext();
const auto *FD = dyn_cast<FuncDecl>(D);
if (!FD)
return true;
ResolvedMemberResult Result = resolveValueMember(*Target->getDeclContext(),
BaseTy,
FD->getEffectiveFullName());
return !(Result.hasBestOverload() && Result.getBestOverload() != D);
bool ShouldPrintChecker::shouldPrint(const Pattern *P, PrintOptions &Options) {
bool ShouldPrint = false;
P->forEachVariable([&](const VarDecl *VD) {
ShouldPrint |= shouldPrint(VD, Options);
});
return ShouldPrint;
}

bool swift::shouldPrint(const Decl *D, PrintOptions &Options) {
if (!shouldPrintAsFavorable(D, Options))
return false;
bool ShouldPrintChecker::shouldPrint(const Decl *D, PrintOptions &Options) {
if (auto *ED= dyn_cast<ExtensionDecl>(D)) {
if (Options.printExtensionContentAsMembers(ED))
return false;
Expand Down Expand Up @@ -1358,7 +1329,7 @@ bool swift::shouldPrint(const Decl *D, PrintOptions &Options) {
if (auto *PD = dyn_cast<PatternBindingDecl>(D)) {
auto ShouldPrint = false;
for (auto entry : PD->getPatternList()) {
ShouldPrint |= shouldPrintPattern(entry.getPattern(), Options);
ShouldPrint |= shouldPrint(entry.getPattern(), Options);
if (ShouldPrint)
return true;
}
Expand All @@ -1368,7 +1339,7 @@ bool swift::shouldPrint(const Decl *D, PrintOptions &Options) {
}

bool PrintAST::shouldPrint(const Decl *D, bool Notify) {
auto Result = swift::shouldPrint(D, Options);
auto Result = Options.shouldPrint(D);
if (!Result && Notify)
Printer.callAvoidPrintDeclPost(D);
return Result;
Expand Down Expand Up @@ -3600,7 +3571,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
};

printFunctionExtInfo(T->getExtInfo());

// If we're stripping argument labels from types, do it when printing.
Type inputType = T->getInput();
if (auto tupleTy = dyn_cast<TupleType>(inputType.getPointer())) {
Expand All @@ -3614,15 +3585,15 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
bool needsParens =
!isa<ParenType>(inputType.getPointer()) &&
!inputType->is<TupleType>();

if (needsParens)
Printer << "(";

visit(inputType);

if (needsParens)
Printer << ")";

if (T->throws())
Printer << " " << tok::kw_throws;

Expand Down Expand Up @@ -3653,7 +3624,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
bool needsParens =
!isa<ParenType>(T->getInput().getPointer()) &&
!T->getInput()->is<TupleType>();

if (needsParens)
Printer << "(";

Expand Down Expand Up @@ -3742,7 +3713,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
PrintOptions subOptions = Options;
subOptions.GenericEnv = nullptr;
TypePrinter sub(Printer, subOptions);

// Capture list used here to ensure we don't print anything using `this`
// printer, but only the sub-Printer.
[&sub, T]{
Expand All @@ -3764,7 +3735,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
sub.Printer << " }";
}();
}

// The arguments to the layout, if any, do come from the outer environment.
if (!T->getGenericArgs().empty()) {
Printer << " <";
Expand Down
Loading