[Localization] Move localization allocator from DiagEngine to the localization producer

HassanElDesouky · HassanElDesouky · commit 8665e0cb558a · 2021-03-23T10:41:54.000+02:00
diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h
@@ -29,7 +29,6 @@
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/StringSaver.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/VersionTuple.h"
 
@@ -725,11 +724,6 @@ namespace swift {
     /// diagnostic message.
     std::unique_ptr<diag::LocalizationProducer> localization;
 
-    /// This allocator will retain localized diagnostic strings containing the
-    /// diagnostic's message and identifier for the duration of compiler invocation.
-    llvm::BumpPtrAllocator localizationAllocator;
-    llvm::StringSaver localizationSaver;
-
     /// The number of open diagnostic transactions. Diagnostics are only
     /// emitted once all transactions have closed.
     unsigned TransactionCount = 0;
@@ -754,8 +748,7 @@ namespace swift {
   public:
     explicit DiagnosticEngine(SourceManager &SourceMgr)
         : SourceMgr(SourceMgr), ActiveDiagnostic(),
-          TransactionStrings(TransactionAllocator),
-          localizationSaver(localizationAllocator) {}
+          TransactionStrings(TransactionAllocator) {}
 
     /// hadAnyError - return true if any *error* diagnostics have been emitted.
     bool hadAnyError() const { return state.hadAnyError(); }
@@ -815,16 +808,15 @@ namespace swift {
       if (llvm::sys::fs::exists(filePath)) {
         if (auto file = llvm::MemoryBuffer::getFile(filePath)) {
           localization = std::make_unique<diag::SerializedLocalizationProducer>(
-              std::move(file.get()), localizationSaver,
-              getPrintDiagnosticNames());
+              std::move(file.get()), getPrintDiagnosticNames());
         }
       } else {
         llvm::sys::path::replace_extension(filePath, ".yaml");
         // In case of missing localization files, we should fallback to messages
         // from `.def` files.
         if (llvm::sys::fs::exists(filePath)) {
           localization = std::make_unique<diag::YAMLLocalizationProducer>(
-              filePath.str(), localizationSaver, getPrintDiagnosticNames());
+              filePath.str(), getPrintDiagnosticNames());
         }
       }
     }
diff --git a/include/swift/Localization/LocalizationFormat.h b/include/swift/Localization/LocalizationFormat.h
@@ -155,14 +155,16 @@ class SerializedLocalizationWriter {
 };
 
 class LocalizationProducer {
-  llvm::Optional<llvm::StringSaver> localizationSaver;
+  /// This allocator will retain localized diagnostic strings containing the
+  /// diagnostic's message and identifier for the duration of compiler
+  /// invocation
+  llvm::BumpPtrAllocator localizationAllocator;
+  llvm::StringSaver localizationSaver;
   bool printDiagnosticName;
 
 public:
-  LocalizationProducer(llvm::Optional<llvm::StringSaver> localizationSaver =
-                           llvm::Optional<llvm::StringSaver>(),
-                       bool printDiagnosticName = false)
-      : localizationSaver(localizationSaver),
+  LocalizationProducer(bool printDiagnosticName = false)
+      : localizationSaver(localizationAllocator),
         printDiagnosticName(printDiagnosticName) {}
 
   /// If the  message isn't available/localized in current context
@@ -186,8 +188,6 @@ class YAMLLocalizationProducer final : public LocalizationProducer {
   std::vector<std::string> unknownIDs;
   explicit YAMLLocalizationProducer(
       llvm::StringRef filePath,
-      llvm::Optional<llvm::StringSaver> localizationSaver =
-          llvm::Optional<llvm::StringSaver>(),
       bool printDiagnosticName = false);
 
   /// Iterate over all of the available (non-empty) translations
@@ -210,8 +210,6 @@ class SerializedLocalizationProducer final : public LocalizationProducer {
 public:
   explicit SerializedLocalizationProducer(
       std::unique_ptr<llvm::MemoryBuffer> buffer,
-      llvm::Optional<llvm::StringSaver> localizationSaver =
-          llvm::Optional<llvm::StringSaver>(),
       bool printDiagnosticName = false);
 
 protected:
diff --git a/lib/Localization/LocalizationFormat.cpp b/lib/Localization/LocalizationFormat.cpp
@@ -99,20 +99,16 @@ LocalizationProducer::getMessageOr(swift::DiagID id,
     return defaultMessage;
   llvm::StringRef diagnosticName(diagnosticNameStrings[(unsigned)id]);
   if (printDiagnosticName) {
-    assert(localizationSaver);
     auto localizedDebugDiagnosticMessage =
-        localizationSaver->save(localizedMessage.str() + diagnosticName.str());
+        localizationSaver.save(localizedMessage.str() + diagnosticName.str());
     return localizedDebugDiagnosticMessage;
   }
   return localizedMessage;
 }
 
 SerializedLocalizationProducer::SerializedLocalizationProducer(
-    std::unique_ptr<llvm::MemoryBuffer> buffer,
-    llvm::Optional<llvm::StringSaver> localizationSaver,
-    bool printDiagnosticName)
-    : LocalizationProducer(localizationSaver, printDiagnosticName),
-      Buffer(std::move(buffer)) {
+    std::unique_ptr<llvm::MemoryBuffer> buffer, bool printDiagnosticName)
+    : LocalizationProducer(printDiagnosticName), Buffer(std::move(buffer)) {
   auto base =
       reinterpret_cast<const unsigned char *>(Buffer.get()->getBufferStart());
   auto tableOffset = endian::read<offset_type>(base, little);
@@ -128,11 +124,9 @@ SerializedLocalizationProducer::getMessage(swift::DiagID id) const {
   return {(const char *)value.getDataPtr(), value.getDataLen()};
 }
 
-YAMLLocalizationProducer::YAMLLocalizationProducer(
-    llvm::StringRef filePath,
-    llvm::Optional<llvm::StringSaver> localizationSaver,
-    bool printDiagnosticName)
-    : LocalizationProducer(localizationSaver, printDiagnosticName) {
+YAMLLocalizationProducer::YAMLLocalizationProducer(llvm::StringRef filePath,
+                                                   bool printDiagnosticName)
+    : LocalizationProducer(printDiagnosticName) {
   auto FileBufOrErr = llvm::MemoryBuffer::getFileOrSTDIN(filePath);
   llvm::MemoryBuffer *document = FileBufOrErr->get();
   diag::LocalizationInput yin(document->getBuffer());
diff --git a/test/diagnostics/Localization/en_debug_diagnostic_name.swift b/test/diagnostics/Localization/en_debug_diagnostic_name.swift
@@ -0,0 +1,13 @@
+// RUN: not %target-swift-frontend -debug-diagnostic-names -localization-path %S/Inputs -locale en -typecheck %s 2>&1 | %FileCheck %s --check-prefix=CHECK_NAMES
+
+_ = "HI!
+// CHECK_NAMES: error: unterminated string literal [lex_unterminated_string]{{$}}
+
+var self1 = self1
+// CHECK_NAMES: error: circular reference [circular_reference]{{$}}
+// CHECK_NAMES: note: through reference here [circular_reference_through]{{$}}
+
+struct Broken {
+  var b : Bool = True 
+}
+// CHECK_NAMES: error: cannot find 'True' in scope [cannot_find_in_scope]{{$}}
diff --git a/test/diagnostics/Localization/en_localization.swift b/test/diagnostics/Localization/en_localization.swift
@@ -1,18 +1,12 @@
-// RUN: not %target-swift-frontend -typecheck %s 2>&1 -localization-path %S/Inputs -locale en | %FileCheck %s --check-prefix=CHECK_NONAMES
-// RUN: not %target-swift-frontend -debug-diagnostic-names -localization-path %S/Inputs -locale en -typecheck %s 2>&1 | %FileCheck %s --check-prefix=CHECK_NAMES
+// RUN: %target-typecheck-verify-swift -localization-path %S/Inputs -locale en
 
 _ = "HI!
-// CHECK_NONAMES: error: unterminated string literal{{$}}
-// CHECK_NAMES: error: unterminated string literal [lex_unterminated_string]{{$}}
-
+// expected-error@-1{{unterminated string literal}}
 var self1 = self1
-// CHECK_NONAMES: error: circular reference{{$}}
-// CHECK_NONAMES: note: through reference here{{$}}
-// CHECK_NAMES: error: circular reference [circular_reference]{{$}}
-// CHECK_NAMES: note: through reference here [circular_reference_through]{{$}}
+// expected-note@-1 2{{through reference here}}
+// expected-error@-2 {{circular reference}}
 
 struct Broken {
-  var b : Bool = True 
+  var b : Bool = True // expected-error{{cannot find 'True' in scope}}
 }
-// CHECK_NONAMES: error: cannot find 'True' in scope{{$}}
-// CHECK_NAMES: error: cannot find 'True' in scope [cannot_find_in_scope]{{$}}
+var v1 : Int[1 // expected-error {{expected ']' in array type}} expected-note {{to match this opening '['}}
diff --git a/test/diagnostics/Localization/fr_debug_diagnostic_name.swift b/test/diagnostics/Localization/fr_debug_diagnostic_name.swift
@@ -0,0 +1,19 @@
+// RUN: %empty-directory(%t)
+// RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/fr.yaml --output-directory=%t/
+// RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/en.yaml --output-directory=%t/ 2>&1 | %FileCheck %s
+// RUN: not %target-swift-frontend -debug-diagnostic-names -localization-path %S/Inputs -locale fr -typecheck %s 2>&1 | %FileCheck %s --check-prefix=CHECK_NAMES
+
+// CHECK: These diagnostic IDs are no longer availiable: 'not_available_in_def, not_available_in_def_2, not_available_in_def_3, not_available_in_def_4, not_available_in_def_5'
+_ = "HI!
+// CHECK_NAMES: error: chaîne non terminée littérale [lex_unterminated_string]{{$}}
+
+// FIXME: This used to produce a localized diagnostic.
+
+var self1 = self1
+// CHECK_NAMES: error: circular reference [circular_reference]{{$}}
+// CHECK_NAMES: note: through reference here [circular_reference_through]{{$}}
+
+struct Broken {
+  var b : Bool = True
+}
+// CHECK_NAMES: error: impossible de trouver 'True' portée [cannot_find_in_scope]{{$}}
diff --git a/test/diagnostics/Localization/fr_localization.swift b/test/diagnostics/Localization/fr_localization.swift
@@ -1,24 +1,18 @@
 // RUN: %empty-directory(%t)
 // RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/fr.yaml --output-directory=%t/
 // RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/en.yaml --output-directory=%t/ 2>&1 | %FileCheck %s
-// RUN: not %target-swift-frontend -typecheck %s 2>&1 -localization-path %S/Inputs -locale fr | %FileCheck %s --check-prefix=CHECK_NONAMES
-// RUN: not %target-swift-frontend -debug-diagnostic-names -localization-path %S/Inputs -locale fr -typecheck %s 2>&1 | %FileCheck %s --check-prefix=CHECK_NAMES
+// RUN: %target-typecheck-verify-swift -localization-path %t -locale fr
 
 // CHECK: These diagnostic IDs are no longer availiable: 'not_available_in_def, not_available_in_def_2, not_available_in_def_3, not_available_in_def_4, not_available_in_def_5'
 _ = "HI!
-// CHECK_NONAMES: error: chaîne non terminée littérale{{$}}
-// CHECK_NAMES: error: chaîne non terminée littérale [lex_unterminated_string]{{$}}
+// expected-error@-1{{chaîne non terminée littérale}}
 
 // FIXME: This used to produce a localized diagnostic.
 
-var self1 = self1
-// CHECK_NONAMES: error: circular reference{{$}}
-// CHECK_NONAMES: note: through reference here{{$}}
-// CHECK_NAMES: error: circular reference [circular_reference]{{$}}
-// CHECK_NAMES: note: through reference here [circular_reference_through]{{$}}
+var self1 = self1 // expected-note 2{{through reference here}}
+// expected-error@-1 {{circular reference}}
 
 struct Broken {
-  var b : Bool = True
+  var b : Bool = True // expected-error{{impossible de trouver 'True' portée}}
 }
-// CHECK_NONAMES: error: impossible de trouver 'True' portée{{$}}
-// CHECK_NAMES: error: impossible de trouver 'True' portée [cannot_find_in_scope]{{$}}
+var v1 : Int[1 // expected-error {{expected ']' in array type}} expected-note {{to match this opening '['}}
