Skip to content

Commit 5c785d4

Browse files
committed
Enforce that FileUnit + LoadedFile have trivial destructors
We already do this for other ASTContext-allocated types (see Decl.cpp). This will prevent the sort of mistakes in the previous two commits. Note that if any particular subclass of FileUnit wants to have its destructor run, it can opt into that manually using ASTContext::addDestructorCleanup. SourceFile and BuiltinUnit both do this. But we generally don't /want/ to do this if we can avoid it because it adds to compiler teardown time.
1 parent 764e2b8 commit 5c785d4

File tree

7 files changed

+21
-15
lines changed

7 files changed

+21
-15
lines changed

include/swift/AST/Module.h

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,10 @@ static inline unsigned alignOfFileUnit();
644644
/// FileUnit is an abstract base class; its subclasses represent different
645645
/// sorts of containers that can each provide a set of decls, e.g. a source
646646
/// file. A module can contain several file-units.
647+
#pragma clang diagnostic push
648+
#pragma clang diagnostic ignored "-Wnon-virtual-dtor"
647649
class FileUnit : public DeclContext {
650+
#pragma clang diagnostic pop
648651
virtual void anchor();
649652

650653
// FIXME: Stick this in a PointerIntPair.
@@ -655,8 +658,6 @@ class FileUnit : public DeclContext {
655658
: DeclContext(DeclContextKind::FileUnit, &M), Kind(kind) {
656659
}
657660

658-
virtual ~FileUnit() = default;
659-
660661
public:
661662
FileUnitKind getKind() const {
662663
return Kind;
@@ -891,13 +892,7 @@ class FileUnit : public DeclContext {
891892
// Make placement new and vanilla new/delete illegal for FileUnits.
892893
void *operator new(size_t Bytes) throw() = delete;
893894
void *operator new(size_t Bytes, void *Mem) throw() = delete;
894-
895-
protected:
896-
// Unfortunately we can't remove this altogether because the virtual
897-
// destructor requires it to be accessible.
898-
void operator delete(void *Data) throw() {
899-
llvm_unreachable("Don't use operator delete on a SourceFile");
900-
}
895+
void operator delete(void *Data) throw() = delete;
901896

902897
public:
903898
// Only allow allocation of FileUnits using the allocator in ASTContext
@@ -1380,9 +1375,11 @@ class BuiltinUnit final : public FileUnit {
13801375
};
13811376

13821377
/// Represents an externally-loaded file of some kind.
1378+
#pragma clang diagnostic push
1379+
#pragma clang diagnostic ignored "-Wnon-virtual-dtor"
13831380
class LoadedFile : public FileUnit {
1381+
#pragma clang diagnostic pop
13841382
protected:
1385-
~LoadedFile() = default;
13861383
LoadedFile(FileUnitKind Kind, ModuleDecl &M) noexcept
13871384
: FileUnit(Kind, M) {
13881385
assert(classof(this) && "invalid kind");

include/swift/ClangImporter/ClangModule.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ class ClangModuleUnit final : public LoadedFile {
3939
/// The metadata of the underlying Clang module.
4040
clang::ExternalASTSource::ASTSourceDescriptor ASTSourceDescriptor;
4141

42-
~ClangModuleUnit() = default;
43-
4442
public:
4543
/// True if the given Module contains an imported Clang module unit.
4644
static bool hasClangModule(ModuleDecl *M);

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,6 @@ class SerializedASTFile final : public LoadedFile {
253253

254254
bool IsSIB;
255255

256-
~SerializedASTFile() = default;
257-
258256
SerializedASTFile(ModuleDecl &M, ModuleFile &file, bool isSIB = false)
259257
: LoadedFile(FileUnitKind::SerializedAST, M), File(file), IsSIB(isSIB) {}
260258

lib/AST/Module.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@
5555

5656
using namespace swift;
5757

58+
static_assert(IsTriviallyDestructible<FileUnit>::value,
59+
"FileUnits are BumpPtrAllocated; the d'tor may not be called");
60+
static_assert(IsTriviallyDestructible<LoadedFile>::value,
61+
"LoadedFiles are BumpPtrAllocated; the d'tor may not be called");
62+
5863
//===----------------------------------------------------------------------===//
5964
// Builtin Module Name lookup
6065
//===----------------------------------------------------------------------===//

lib/ClangImporter/ClangImporter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3176,6 +3176,9 @@ void ClangImporter::verifyAllModules() {
31763176
// ClangModule Implementation
31773177
//===----------------------------------------------------------------------===//
31783178

3179+
static_assert(IsTriviallyDestructible<ClangModuleUnit>::value,
3180+
"ClangModuleUnits are BumpPtrAllocated; the d'tor is not called");
3181+
31793182
ClangModuleUnit::ClangModuleUnit(ModuleDecl &M,
31803183
ClangImporter::Implementation &owner,
31813184
const clang::Module *clangModule)

lib/ClangImporter/DWARFImporter.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ void DWARFImporterDelegate::anchor() {}
2020
/// Represents a Clang module that was "imported" from debug info. Since all the
2121
/// loading of types is done on demand, this class is effectively empty.
2222
class DWARFModuleUnit final : public LoadedFile {
23-
~DWARFModuleUnit() = default;
2423
ClangImporter::Implementation &Owner;
2524

2625
public:
@@ -96,6 +95,9 @@ class DWARFModuleUnit final : public LoadedFile {
9695
}
9796
};
9897

98+
static_assert(IsTriviallyDestructible<DWARFModuleUnit>::value,
99+
"DWARFModuleUnits are BumpPtrAllocated; the d'tor is not called");
100+
99101
ModuleDecl *ClangImporter::Implementation::loadModuleDWARF(
100102
SourceLoc importLoc, ArrayRef<std::pair<Identifier, SourceLoc>> path) {
101103
// There's no importing from debug info if no importer is installed.

lib/Serialization/ModuleFile.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ using namespace swift::serialization;
3838
using namespace llvm::support;
3939
using llvm::Expected;
4040

41+
static_assert(IsTriviallyDestructible<SerializedASTFile>::value,
42+
"SerializedASTFiles are BumpPtrAllocated; d'tors are not called");
43+
4144
static bool checkModuleSignature(llvm::BitstreamCursor &cursor,
4245
ArrayRef<unsigned char> signature) {
4346
for (unsigned char byte : signature)

0 commit comments

Comments
 (0)