Improve diagnostics for uses of unsafe declarations in functions

Instead of producing a warning for each use of an unsafe entity,
collect all of the uses of unsafe constructs within a given function
and batch them together in a single diagnostic at the function level
that tells you what you can do (add `@unsafe` or `@safe(unchecked)`,
depending on whether all unsafe uses were in the definition), plus
notes identifying every unsafe use within that declaration. The new
diagnostic renderer nicely collects together in a single snippet, so
it's easier to reason about.

Here's an example from the embedded runtime that previously would have
been 6 separate warnings, each with 1-2 notes:

```
swift/stdlib/public/core/EmbeddedRuntime.swift:397:13: warning: global function 'swift_retainCount' involves unsafe code; use '@safe(unchecked)' to assert that the code is memory-safe
395 |
396 | @_cdecl("swift_retainCount")
397 | public func swift_retainCount(object: Builtin.RawPointer) -> Int {
    |             `- warning: global function 'swift_retainCount' involves unsafe code; use '@safe(unchecked)' to assert that the code is memory-safe
398 |   if !isValidPointerForNativeRetain(object: object) { return 0 }
399 |   let o = UnsafeMutablePointer<HeapObject>(object)
    |           |                              `- note: call to unsafe initializer 'init(_:)'
    |           `- note: reference to unsafe generic struct 'UnsafeMutablePointer'
400 |   let refcount = refcountPointer(for: o)
    |                  |                    `- note: reference to let 'o' involves unsafe type 'UnsafeMutablePointer<HeapObject>'
    |                  `- note: call to global function 'refcountPointer(for:)' involves unsafe type 'UnsafeMutablePointer<Int>'
401 |   return loadAcquire(refcount) & HeapObject.refcountMask
    |          |           `- note: reference to let 'refcount' involves unsafe type 'UnsafeMutablePointer<Int>'
    |          `- note: call to global function 'loadAcquire' involves unsafe type 'UnsafeMutablePointer<Int>'
402 | }
403 |
```

Note that we have lost a little bit of information, because we no
longer produce "unsafe declaration was here" notes pointing back at
things like `UnsafeMutablePointer` or `recountPointer(for:)`. However,
strict memory safety tends to be noisy to turn on, so it's worth
losing a little bit of easily-recovered information to gain some
brevity.
This commit is contained in:
Doug Gregor
2024-12-19 09:50:08 -08:00
parent b3763eed7b
commit 29f23bb66a
15 changed files with 231 additions and 55 deletions

View File

@@ -41,6 +41,7 @@
#include "swift/AST/PrintOptions.h"
#include "swift/AST/ProtocolConformance.h"
#include "swift/AST/SourceFile.h"
#include "swift/AST/SourceFileExtras.h"
#include "swift/AST/SynthesizedFileUnit.h"
#include "swift/AST/Type.h"
#include "swift/AST/TypeCheckRequests.h"
@@ -1175,6 +1176,17 @@ ASTNode SourceFile::getNodeInEnclosingSourceFile() const {
return ASTNode::getFromOpaqueValue(getGeneratedSourceFileInfo()->astNode);
}
SourceFileExtras &SourceFile::getExtras() const {
if (!extras) {
const_cast<SourceFile *>(this)->extras = new SourceFileExtras();
getASTContext().addCleanup([extras=this->extras] {
delete extras;
});
}
return *extras;
}
void ModuleDecl::lookupClassMember(ImportPath::Access accessPath,
DeclName name,
SmallVectorImpl<ValueDecl*> &results) const {