From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Thu, 28 Jun 2018 01:57:04 +0000 (+0000)
Subject: [modules] Do not serialize / deserialize pending new/delete mismatch
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4dc3dee0b40da5a342d04142934d6318af523c27;p=clang

[modules] Do not serialize / deserialize pending new/delete mismatch
checks across module boundaries. This was causing us to load constructor
definitions for all consumers of a module with a pending check.

(In one case we saw ~7% of total frontend time spent loading
constructors for this check.)

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335807 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 7a8cd1ca4e..d6d105c3b0 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -1010,11 +1010,6 @@ void Sema::ActOnEndOfTranslationUnit() {
     // Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
     // modules when they are built, not every time they are used.
     emitAndClearUnusedLocalTypedefWarnings();
-
-    // Modules don't need any of the checking below.
-    if (!PP.isIncrementalProcessingEnabled())
-      TUScope = nullptr;
-    return;
   }
 
   // C99 6.9.2p2:
@@ -1032,8 +1027,7 @@ void Sema::ActOnEndOfTranslationUnit() {
   for (TentativeDefinitionsType::iterator
             T = TentativeDefinitions.begin(ExternalSource),
          TEnd = TentativeDefinitions.end();
-       T != TEnd; ++T)
-  {
+       T != TEnd; ++T) {
     VarDecl *VD = (*T)->getActingDefinition();
 
     // If the tentative definition was completed, getActingDefinition() returns
@@ -1060,12 +1054,13 @@ void Sema::ActOnEndOfTranslationUnit() {
     // Notify the consumer that we've completed a tentative definition.
     if (!VD->isInvalidDecl())
       Consumer.CompleteTentativeDefinition(VD);
-
   }
 
   // If there were errors, disable 'unused' warnings since they will mostly be
-  // noise.
-  if (!Diags.hasErrorOccurred()) {
+  // noise. Don't warn for a use from a module: either we should warn on all
+  // file-scope declarations in modules or not at all, but whether the
+  // declaration is used is immaterial.
+  if (!Diags.hasErrorOccurred() && TUKind != TU_Module) {
     // Output warning for unused file scoped decls.
     for (UnusedFileScopedDeclsType::iterator
            I = UnusedFileScopedDecls.begin(ExternalSource),
@@ -1133,6 +1128,8 @@ void Sema::ActOnEndOfTranslationUnit() {
   }
 
   if (!Diags.isIgnored(diag::warn_unused_private_field, SourceLocation())) {
+    // FIXME: Load additional unused private field candidates from the external
+    // source.
     RecordCompleteMap RecordsComplete;
     RecordCompleteMap MNCComplete;
     for (NamedDeclSetType::iterator I = UnusedPrivateFields.begin(),
diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 7d5b521d4d..1c838ae2c0 100644
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -4769,13 +4769,15 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
   // analyze later in AST.
   RecordData DeleteExprsToAnalyze;
 
-  for (const auto &DeleteExprsInfo :
-       SemaRef.getMismatchingDeleteExpressions()) {
-    AddDeclRef(DeleteExprsInfo.first, DeleteExprsToAnalyze);
-    DeleteExprsToAnalyze.push_back(DeleteExprsInfo.second.size());
-    for (const auto &DeleteLoc : DeleteExprsInfo.second) {
-      AddSourceLocation(DeleteLoc.first, DeleteExprsToAnalyze);
-      DeleteExprsToAnalyze.push_back(DeleteLoc.second);
+  if (!isModule) {
+    for (const auto &DeleteExprsInfo :
+         SemaRef.getMismatchingDeleteExpressions()) {
+      AddDeclRef(DeleteExprsInfo.first, DeleteExprsToAnalyze);
+      DeleteExprsToAnalyze.push_back(DeleteExprsInfo.second.size());
+      for (const auto &DeleteLoc : DeleteExprsInfo.second) {
+        AddSourceLocation(DeleteLoc.first, DeleteExprsToAnalyze);
+        DeleteExprsToAnalyze.push_back(DeleteLoc.second);
+      }
     }
   }
 
diff --git a/test/Modules/new-delete.cpp b/test/Modules/new-delete.cpp
new file mode 100644
index 0000000000..585a242b22
--- /dev/null
+++ b/test/Modules/new-delete.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fmodules -verify %s
+// expected-no-diagnostics
+
+#pragma clang module build M
+module M {}
+#pragma clang module contents
+#pragma clang module begin M
+struct A {
+  A();
+  ~A() { delete p; } // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'}}
+  int *p;
+};
+inline A::A() : p(new int[32]) {} // expected-note {{allocated}}
+struct B {
+  B();
+  ~B() { delete p; }
+  int *p;
+};
+#pragma clang module end
+#pragma clang module endbuild
+
+#pragma clang module import M
+B::B() : p(new int[32]) {}