Skip to content

Serialize whether VarDecls are top-level #29489

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

Closed
wants to merge 9 commits into from
Closed

Serialize whether VarDecls are top-level #29489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
898346f
[CSFix] Add a fix to add a missing qualifier to shadowed top-level na…
xedin Jan 4, 2020
57ff839
[Diagnostics] Port name shadowing diagnostics
xedin Jan 4, 2020
0a012b2
[ConstraintSystem] Use new fix/diagnostic for name shadowing
xedin Jan 6, 2020
b5b8691
[CSDiag] NFC: Remove obsolete name shadowing diagnostics
xedin Jan 6, 2020
c240093
[Diagnostics] NFC: Fix name shadowing diagnostic comment to refer to …
xedin Jan 6, 2020
e75de93
[TypeChecker] Add types nested in protocol to lookup results
xedin Jan 7, 2020
a28c8cd
Merge remote-tracking branch 'xedin/diagnose-shadowing'
adrian-prantl Jan 28, 2020
7d9dfbf
Serialize whether a VarDecl is a top-level global.
adrian-prantl Jan 28, 2020
93dd4df
Compile libraries in testcases with -parse-as-library (NFC)
adrian-prantl Jan 29, 2020
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
11 changes: 9 additions & 2 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ class alignas(1 << DeclAlignInBits) Decl {
IsStatic : 1
);

SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1+1,
SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1+1+1,
/// Encodes whether this is a 'let' binding.
Introducer : 1,

Expand All @@ -358,7 +358,10 @@ class alignas(1 << DeclAlignInBits) Decl {
IsLazyStorageProperty : 1,

/// Whether this is the backing storage for a property wrapper.
IsPropertyWrapperBackingProperty : 1
IsPropertyWrapperBackingProperty : 1,

/// Whether this is a lazily top-level global variable from the main file.
IsTopLevelGlobal : 1
);

SWIFT_INLINE_BITFIELD(ParamDecl, VarDecl, 1+2+NumDefaultArgumentKindBits,
Expand Down Expand Up @@ -5084,6 +5087,10 @@ class VarDecl : public AbstractStorageDecl {
Bits.VarDecl.IsLazyStorageProperty = IsLazyStorage;
}

/// True if this is a top-level global variable from the main source file.
bool isTopLevelGlobal() const { return Bits.VarDecl.IsTopLevelGlobal; }
void setTopLevelGlobal(bool b) { Bits.VarDecl.IsTopLevelGlobal = b; }

/// Retrieve the custom attributes that attach property wrappers to this
/// property. The returned list contains all of the attached property wrapper attributes in source order,
/// which means the outermost wrapper attribute is provided first.
Expand Down
12 changes: 5 additions & 7 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,14 +1138,12 @@ NOTE(candidate_expected_different_labels,none,
"incorrect labels for candidate (have: '%0', expected: '%1')",
(StringRef, StringRef))

ERROR(member_shadows_function,none,
"use of %0 refers to %1 rather than %2 %3",
(DeclNameRef, DescriptiveDeclKind, DescriptiveDeclKind, DeclName))
ERROR(member_shadows_global_function,none,
"use of %0 refers to %1 %2 rather than %3 %4 in %5 %6",
(DeclNameRef, DescriptiveDeclKind, DeclName, DescriptiveDeclKind,
DeclName, DescriptiveDeclKind, DeclName))
ERROR(member_shadows_global_function_near_match,none,
"use of %0 nearly matches %3 %4 in %5 %6 rather than %1 %2",
(DeclNameRef, DescriptiveDeclKind, DeclName, DescriptiveDeclKind,
DeclName, DescriptiveDeclKind, DeclName))
"use of %0 refers to %1 rather than %2 %3 in module %4",
(DeclNameRef, DescriptiveDeclKind, DescriptiveDeclKind, DeclName, DeclName))

ERROR(instance_member_use_on_type,none,
"instance member %1 cannot be used on type %0; "
Expand Down
7 changes: 2 additions & 5 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5312,6 +5312,7 @@ VarDecl::VarDecl(DeclKind kind, bool isStatic, VarDecl::Introducer introducer,
Bits.VarDecl.IsLazyStorageProperty = false;
Bits.VarDecl.HasNonPatternBindingInit = false;
Bits.VarDecl.IsPropertyWrapperBackingProperty = false;
Bits.VarDecl.IsTopLevelGlobal = false;
}

Type VarDecl::getType() const {
Expand Down Expand Up @@ -5410,11 +5411,7 @@ bool VarDecl::isLazilyInitializedGlobal() const {

// Top-level global variables in the main source file and in the REPL are not
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the above checks (hasClangNode() and isDebuggerVar()) needed at all then? The bit should not be set in those cases either.

// lazily initialized.
auto sourceFileContext = dyn_cast<SourceFile>(getDeclContext());
if (!sourceFileContext)
return true;

return !sourceFileContext->isScriptMode();
return !isTopLevelGlobal();
}

SourceRange VarDecl::getSourceRange() const {
Expand Down
1 change: 1 addition & 0 deletions lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5840,6 +5840,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
VD->setStatic(StaticLoc.isValid());
VD->getAttrs() = Attributes;
setLocalDiscriminator(VD);
VD->setTopLevelGlobal(topLevelDecl);

// Set original declaration in `@differentiable` attributes.
setOriginalDeclarationForDifferentiableAttributes(Attributes, VD);
Expand Down
225 changes: 0 additions & 225 deletions lib/Sema/CSDiag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
ContextualTypePurpose CTP,
Type suggestedType = Type());

bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
CalleeCandidateInfo &CCI,
ArrayRef<Identifier> argLabels);

private:
/// Validate potential contextual type for type-checking one of the
/// sub-expressions, usually correct/valid types are the ones which
Expand Down Expand Up @@ -964,223 +960,6 @@ decomposeArgType(Type argType, ArrayRef<Identifier> argLabels) {
return result;
}

bool FailureDiagnosis::diagnoseImplicitSelfErrors(
Expr *fnExpr, Expr *argExpr, CalleeCandidateInfo &CCI,
ArrayRef<Identifier> argLabels) {
// If candidate list is empty it means that problem is somewhere else,
// since we need to have candidates which might be shadowing other funcs.
if (CCI.empty() || !CCI[0].getDecl())
return false;

auto &ctx = CS.getASTContext();
// Call expression is formed as 'foo.bar' where 'foo' might be an
// implicit "Self" reference, such use wouldn't provide good diagnostics
// for situations where instance members have equal names to functions in
// Swift Standard Library e.g. min/max.
auto UDE = dyn_cast<UnresolvedDotExpr>(fnExpr);
if (!UDE)
return false;

auto baseExpr = dyn_cast<DeclRefExpr>(UDE->getBase());
if (!baseExpr)
return false;

auto baseDecl = baseExpr->getDecl();
if (!baseExpr->isImplicit() || baseDecl->getFullName() != ctx.Id_self)
return false;

// Our base expression is an implicit 'self.' reference e.g.
//
// extension Sequence {
// func test() -> Int {
// return max(1, 2)
// }
// }
//
// In this example the Sequence class already has two methods named 'max'
// none of which accept two arguments, but there is a function in
// Swift Standard Library called 'max' which does accept two arguments,
// so user might have called that by mistake without realizing that
// compiler would add implicit 'self.' prefix to the call of 'max'.
auto argType = CS.getType(argExpr);
// If argument wasn't properly type-checked, let's retry without changing AST.
if (!argType || argType->hasUnresolvedType() || argType->hasTypeVariable() ||
argType->hasTypeParameter()) {
auto *argTuple = dyn_cast<TupleExpr>(argExpr);
if (!argTuple) {
// Bail out if we don't have a well-formed argument list.
return false;
}

// Let's type check individual argument expressions without any
// contextual information to try to recover an argument type that
// matches what the user actually wrote instead of what the typechecker
// expects.
SmallVector<TupleTypeElt, 4> elts;
for (unsigned i = 0, e = argTuple->getNumElements(); i < e; ++i) {
ConcreteDeclRef ref = nullptr;
auto *el = argTuple->getElement(i);
auto typeResult =
TypeChecker::getTypeOfExpressionWithoutApplying(el, CS.DC, ref);
if (!typeResult)
return false;
auto flags = ParameterTypeFlags().withInOut(typeResult->is<InOutType>());
elts.push_back(TupleTypeElt(typeResult->getInOutObjectType(),
argTuple->getElementName(i),
flags));
}

argType = TupleType::get(elts, CS.getASTContext());
}

auto typeKind = argType->getKind();
if (typeKind != TypeKind::Tuple && typeKind != TypeKind::Paren)
return false;

// If argument type couldn't be properly resolved or has errors,
// we can't diagnose anything in here, it points to the different problem.
if (isUnresolvedOrTypeVarType(argType) || argType->hasError())
return false;

auto context = CS.DC;
using CandidateMap =
llvm::SmallDenseMap<ValueDecl *, llvm::SmallVector<OverloadChoice, 2>>;

auto getBaseKind = [](ValueDecl *base) -> DescriptiveDeclKind {
DescriptiveDeclKind kind = DescriptiveDeclKind::Module;
if (!base)
return kind;

auto context = base->getDeclContext();
do {
if (isa<ExtensionDecl>(context))
return DescriptiveDeclKind::Extension;

if (auto nominal = dyn_cast<NominalTypeDecl>(context)) {
kind = nominal->getDescriptiveKind();
break;
}

context = context->getParent();
} while (context);

return kind;
};

auto diagnoseShadowing = [&](ValueDecl *base,
ArrayRef<OverloadChoice> candidates) -> bool {
CalleeCandidateInfo calleeInfo(base ? base->getInterfaceType() : nullptr,
candidates, CCI.hasTrailingClosure, CS,
base);

calleeInfo.filterListArgs(decomposeArgType(argType, argLabels));

auto diagnostic = diag::member_shadows_global_function_near_match;
switch (calleeInfo.closeness) {
case CC_Unavailable:
case CC_Inaccessible:
case CC_SelfMismatch:
case CC_ArgumentLabelMismatch:
case CC_ArgumentCountMismatch:
case CC_GeneralMismatch:
return false;

case CC_NonLValueInOut:
case CC_OneArgumentNearMismatch:
case CC_OneArgumentMismatch:
case CC_OneGenericArgumentNearMismatch:
case CC_OneGenericArgumentMismatch:
case CC_ArgumentNearMismatch:
case CC_ArgumentMismatch:
case CC_GenericNonsubstitutableMismatch:
break; // Near match cases

case CC_ExactMatch:
diagnostic = diag::member_shadows_global_function;
break;
}

auto choice = calleeInfo.candidates[0].getDecl();
auto baseKind = getBaseKind(base);
auto baseName = getBaseName(choice->getDeclContext());

auto origCandidate = CCI[0].getDecl();
ctx.Diags.diagnose(UDE->getLoc(), diagnostic, UDE->getName(),
origCandidate->getDescriptiveKind(),
origCandidate->getFullName(),
choice->getDescriptiveKind(),
choice->getFullName(), baseKind, baseName);

auto topLevelDiag = diag::fix_unqualified_access_top_level;
if (baseKind == DescriptiveDeclKind::Module)
topLevelDiag = diag::fix_unqualified_access_top_level_multi;

emitFixItForExplicitlyQualifiedReference(ctx.Diags, UDE, topLevelDiag,
baseName,
choice->getDescriptiveKind());

for (auto &candidate : calleeInfo.candidates) {
if (auto decl = candidate.getDecl())
ctx.Diags.diagnose(decl, diag::decl_declared_here, decl->getFullName());
}

return true;
};

// For each of the parent contexts, let's try to find any candidates
// which have the same name and the same number of arguments as callee.
while (context->getParent()) {
auto result =
TypeChecker::lookupUnqualified(context, UDE->getName(), UDE->getLoc());
context = context->getParent();

if (!result || result.empty())
continue;

CandidateMap candidates;
for (const auto &candidate : result) {
auto base = candidate.getBaseDecl();
auto decl = candidate.getValueDecl();
if ((base && base->isInvalid()) || decl->isInvalid())
continue;

// If base is present but it doesn't represent a valid nominal,
// we can't use current candidate as one of the choices.
if (base && !base->getInterfaceType()->getNominalOrBoundGenericNominal())
continue;

auto context = decl->getDeclContext();
// We are only interested in static or global functions, because
// there is no way to call anything else properly.
if (!decl->isStatic() && !context->isModuleScopeContext())
continue;

OverloadChoice choice(base ? base->getInterfaceType() : nullptr,
decl, UDE->getFunctionRefKind());

if (base) { // Let's group all of the candidates have a common base.
candidates[base].push_back(choice);
continue;
}

// If there is no base, it means this is one of the global functions,
// let's try to diagnose its shadowing inline.
if (diagnoseShadowing(base, choice))
return true;
}

if (candidates.empty())
continue;

for (const auto &candidate : candidates) {
if (diagnoseShadowing(candidate.getFirst(), candidate.getSecond()))
return true;
}
}

return false;
}

// Extract expression for failed argument number
static Expr *getFailedArgumentExpr(CalleeCandidateInfo CCI, Expr *argExpr) {
if (auto *TE = dyn_cast<TupleExpr>(argExpr))
Expand All @@ -1202,10 +981,6 @@ static Expr *getFailedArgumentExpr(CalleeCandidateInfo CCI, Expr *argExpr) {
bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
Expr *fnExpr, Expr *argExpr,
ArrayRef<Identifier> argLabels) {
// Try to diagnose errors related to the use of implicit self reference.
if (diagnoseImplicitSelfErrors(fnExpr, argExpr, CCI, argLabels))
return true;

// If we have a failure where the candidate set differs on exactly one
// argument, and where we have a consistent mismatch across the candidate set
// (often because there is only one candidate in the set), then diagnose this
Expand Down
42 changes: 42 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6117,3 +6117,45 @@ bool UnableToInferProtocolLiteralType::diagnoseAsError() {

return true;
}

bool MissingQuialifierInMemberRefFailure::diagnoseAsError() {
auto selectedOverload = getOverloadChoiceIfAvailable(getLocator());
if (!selectedOverload)
return false;

auto *UDE = cast<UnresolvedDotExpr>(getRawAnchor());

auto baseType = getType(UDE->getBase());

auto methodKind = baseType->isAnyExistentialType()
? DescriptiveDeclKind::StaticMethod
: DescriptiveDeclKind::Method;

auto choice = selectedOverload->choice.getDeclOrNull();
if (!choice)
return false;

auto *DC = choice->getDeclContext();
if (!(DC->isModuleContext() || DC->isModuleScopeContext())) {
emitDiagnostic(UDE->getLoc(), diag::member_shadows_function, UDE->getName(),
methodKind, choice->getDescriptiveKind(),
choice->getFullName());
return true;
}

auto qualifier = DC->getParentModule()->getName();

emitDiagnostic(UDE->getLoc(), diag::member_shadows_global_function,
UDE->getName(), methodKind, choice->getDescriptiveKind(),
choice->getFullName(), qualifier);

SmallString<32> namePlusDot = qualifier.str();
namePlusDot.push_back('.');

emitDiagnostic(UDE->getLoc(), diag::fix_unqualified_access_top_level_multi,
namePlusDot, choice->getDescriptiveKind(), qualifier)
.fixItInsert(UDE->getStartLoc(), namePlusDot);

emitDiagnostic(choice, diag::decl_declared_here, choice->getFullName());
return true;
}
Loading