Syntactically verify that initializer expressions of '@const' variables and argument expressions to '@const' parameters consist strictly of syntactically-verifiable set of basic values and operations
We previously missed diagnosing this for macro
args, fixing it turned out to be a bit more source
breaking than initially thought though, so downgrade
to a warning until Swift 7.
rdar://141963700
I missed this in my previous PR, but this is needed
to ensure we visit macro arguments for macro expansion
exprs that have substitute MacroExpansionDecls since
we prefer to visit the arguments on the decl once
the expression has been expanded.
Previously we would only run MiscDiagnostics
passes on macro arguments for some statement
diagnostics, update the expression walkers that
inherit from BaseDiagnosticWalker such that we
consistently do MiscDiagnostics on macro arguments.
Since this function is being called from the constraint solver now, we
need to generalize the way it obtains the Type of an Expression, as the
expression itself may not know its own type, only the solver does.
resolves rdar://134371893 / https://github.com/swiftlang/swift/issues/75999
I want to expose the syntactic checking to other parts of Sema, so that
we can determine whether an expression is a candidate for having a
`consume` added to it.
When diagnosing a concurrency-unsafe global or static variable, provide
Fix-Its with specific guidance and advice. This is intended to aid the
workflow for folks enabling strict concurrency checking or Swift 6.
There are up to three Fix-Its attached to a diagnostic about
concurrency-unsafe global/static variables:
* convert 'global' to a 'let' constant to make the shared state
immutable, which replaces `var` with `let`
* restrict 'global' to the main actor if it will only be accessed from the
main thread, which adds `@MainActor`
* unsafely mark %0 as concurrency-safe if all accesses are protected
by an external synchronization mechanism, which adds `nonisolated(unsafe)`
I fretted over two things before deciding on this path:
1. For the second note, the reality is that any global actor will
suffice, but `@MainActor` is orders of magnitude more common than any
other global actor, so "common case convenience" wins over "precise
but less useful.
2. For the third note, `nonisolated(unsafe)` should only be used
sparingly, and surfacing it via Fix-It could cause overuse. However,
developers need to know about it, and this is how we do that. It comes
last in the list of notes (after the better options) and says "unsafe"
in not one but two places.
We can use part of the new infrastructure if we simply handle abstract
conformances to Copyable, which is what we'd get upon lookup for a
nominal in the old world. This means that we can merge diagnostics for
the containment test together, and fix differences with deinit
diagnostics.
Move out-of-place SingleValueStmtExpr checking into
`performSyntacticExprDiagnostics`, to ensure we
catch all expressions. Previously we did the walk
as a part of Decl-based MiscDiagnostics, but it
turns out that can miss expressions in property
initializers, subscript default arguments, and
custom attrs.
This does mean that we'll now no longer diagnose
out-of-place if/switch exprs if the expression
didn't type-check, but that's consistent with the
rest of MiscDiagnostics, and I don't think it will
be a major issue in practice. We probably ought to
consider moving this checking into PreCheckExpr,
but that would require first separating out
SequenceExpr folding, which has other consequences,
and so I'm leaving as future work for now.
This is phase-1 of switching from llvm::Optional to std::optional in the
next rebranch. llvm::Optional was removed from upstream LLVM, so we need
to migrate off rather soon. On Darwin, std::optional, and llvm::Optional
have the same layout, so we don't need to be as concerned about ABI
beyond the name mangling. `llvm::Optional` is only returned from one
function in
```
getStandardTypeSubst(StringRef TypeName,
bool allowConcurrencyManglings);
```
It's the return value, so it should not impact the mangling of the
function, and the layout is the same as `std::optional`, so it should be
mostly okay. This function doesn't appear to have users, and the ABI was
already broken 2 years ago for concurrency and no one seemed to notice
so this should be "okay".
I'm doing the migration incrementally so that folks working on main can
cherry-pick back to the release/5.9 branch. Once 5.9 is done and locked
away, then we can go through and finish the replacement. Since `None`
and `Optional` show up in contexts where they are not `llvm::None` and
`llvm::Optional`, I'm preparing the work now by going through and
removing the namespace unwrapping and making the `llvm` namespace
explicit. This should make it fairly mechanical to go through and
replace llvm::Optional with std::optional, and llvm::None with
std::nullopt. It's also a change that can be brought onto the
release/5.9 with minimal impact. This should be an NFC change.
Provide ASTWalker with a customization point to specify whether to
check macro arguments (which are type checked but never emitted), the
macro expansion (which is the result of applying the macro and is
actually emitted into the source), or both. Provide answers for the
~115 different ASTWalker visitors throughout the code base.
Fixes rdar://104042945, which concerns checking of effects in
macro arguments---which we shouldn't do.
Most of the diagnostics for extra/missing/mislabeled arguments refer
to argument to a "call". Some (but not call) would substitute in
"subscript". None would refer to an argument to a macro expansion
properly.
Rework all of these to refer to the argument in a call, subscript, or
macro expansion as appropriate. Fix up lots of tests that now say
"subscript" instead, and add tests for macro expansions.
Replace the use of bool and pointer returns for
`walkToXXXPre`/`walkToXXXPost`, and instead use
explicit actions such as `Action::Continue(E)`,
`Action::SkipChildren(E)`, and `Action::Stop()`.
There are also conditional variants, e.g
`Action::SkipChildrenIf`, `Action::VisitChildrenIf`,
and `Action::StopIf`.
There is still more work that can be done here, in
particular:
- SourceEntityWalker still needs to be migrated.
- Some uses of `return false` in pre-visitation
methods can likely now be replaced by
`Action::Stop`.
- We still use bool and pointer returns internally
within the ASTWalker traversal, which could likely
be improved.
But I'm leaving those as future work for now as
this patch is already large enough.
For a non-public property where the type is defined by an assignment, like
`internal var internalAssigned = NewStruct()`, type-checking the type's
availability is done via checking the initializer expression.
In -check-api-availaiblity-only mode, pass down a flag to not check
availability in expressions for initializer expressions of such
non-public properties.
rdar://84389825
Instead of a new attribute `@completionHandlerAsync`, allow the use of
the existing `renamed` parameter of `@available` to specify the
asynchronous alternative of a synchronous function.
No errors will be output from invalid names as `@completionHandlerAsync`
had, but if a function is correctly matched then it will be used to
output warnings when using the synchronous function in an asynchronous
context (as before).
Resolves rdar://80612731
We used to diagnose references to unavailable declarations in
two places:
- inside Exprs, right after type checking the expression
- inside TypeReprs, from resolveType()
In broad terms, resolveType() is called with TypeReprs
stored inside both Stmts and Decls.
To handle the first case, I added a new overload of
diagAvailability() that takes a Stmt, to be called from
typeCheckStmt(). This doesn't actually walk into any Exprs
stored inside the statement; this means it only walks
Patterns and such.
For the second case, a new DeclAvailabilityChecker is
now defined in TypeCheckAccess.cpp. It's structure is
analogous to the other three walkers there:
- AccessControlChecker
- UsableFromInlineChecker
- ExportabilityChecker
The new implementation of availability checking for types
introduces a lot more code than the old online logic
it replaces. However, I hope to consolidate some of the
code duplication among the four checkers that are defined
in TypeCheckAccess.cpp, and do some other cleanups that
will make the benefit of the new approach apparent.
We used to take all the captures of a local function and treat them all
as read and write usages of vars from an outer scope. Instead, let's
refactor the analysis to walk into local functions.
Refactor diagnostic emission so it lazily emits notes and fixits.
This is a necessary evil in a world where an arbitrary request can also
emit diagnostics, especially circularity diagnostics. In the future, we
should assert in the evaluator that there is no active diagnostic to
catch the rest of these.
Extend the "type check function body" request to also cover the case
where we have a specific ending source location. Fold all of this
functionality into a single request, so we consistently go through a
request to compute a type-checked function body.
Instead of building ArgumentShuffleExprs, lets just build a TupleExpr,
with explicit representation of collected varargs and default
arguments.
This isn't quite as elegant as it should be, because when re-typechecking,
SanitizeExpr needs to restore the 'old' parameter list by stripping out
the nodes inserted by type checking. However that hackery is all isolated
in one place and will go away soon.
Note that there's a minor change the generated SIL. Caller default
arguments (#file, #line, etc) are no longer delayed and are instead
evaluated in their usual argument position. I don't believe this actually
results in an observable change in behavior, but if it turns out to be
a problem, we can pretty easily change it back to the old behavior with a
bit of extra work.
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
This patch adds warning for redundant access-level modifiers
used in an extension. It also refines the diagnostics of
access_control_ext_member_more issues, in case the fixit
could suggest redundant modifiers.
Resolves: SR-8453.
Pass through the location of the equal '=' token for pattern binding decl entries, and use this location for the immediate deallocation diagnostic. Previously, we were just diagnosing on the start of the initialiser expression.
Additionally, this commit moves the call to `diagnoseUnownedImmediateDeallocation` from `typeCheckBinding` to `typeCheckPatternBinding`. This not only gives us easier access to the PBD entry, but also avoids calling the diagnostic logic for statement conditions such as `if let x = <expr>`. We currently never diagnose on these anyway, as the 'weak' and 'unowned' keywords cannot be applied to such bindings.
Resolves [SR-7340](https://bugs.swift.org/browse/SR-7340).
A type checker instance isn’t needed for most of the “override” checking,
so remove the TypeChecker parameters and instead get the ASTContext or
DiagnosticEngine from whatever decl or type is available. Another step
toward reducing dependency on a TypeChecker instance.
When performing a binding/assignment to a weak or unowned variable/property from an initialiser call, emit a warning that the instance will be immediately deallocated.
"Accessibility" has a different meaning for app developers, so we've
already deliberately excised it from our diagnostics in favor of terms
like "access control" and "access level". Do the same in the compiler
now that we aren't constantly pulling things into the release branch.
Rename AccessibilityAttr to AccessControlAttr and
SetterAccessibilityAttr to SetterAccessAttr, then track down the last
few uses of "accessibility" that don't have to do with
NSAccessibility. (I left the SourceKit XPC API alone because that's
supposed to be more stable.)
