Address feedback

Author: usama54321

include/swift/SIL/SILLocation.h

@@ -52,11 +52,9 @@ class SILLocation {
   /// Describes a position in a source file by explicitly storing the file name,
   /// line and column.
   ///
-  /// This is used for parsed locations from a SIL file, for
+  /// This is used for parsed locations from SIL and swiftmodule files, for
   /// "-sil-based-debuginfo" (see SILDebugInfoGenerator) and for the
   /// "compiler-generated" singleton location.
-  /// In future we might also use it for de-serialized locations from a
-  /// swiftmodule file.
   struct FilenameAndLocation : public SILAllocated<FilenameAndLocation> {
     unsigned line;
     uint16_t column;
@@ -556,9 +554,7 @@ class RegularLocation : public SILLocation {
                   bool isForDebugOnly = true);
   RegularLocation(SourceLoc L, bool Implicit = true)
       : SILLocation(L, RegularKind, Implicit) {}
-  RegularLocation(FilenameAndLocation *filePos)
-    : SILLocation(filePos, RegularKind) {}
-  RegularLocation(FilenameAndLocation *filePos, bool Implicit)
+  RegularLocation(FilenameAndLocation *filePos, bool Implicit = false)
     : SILLocation(filePos, RegularKind, Implicit) {}
 
   /// Convert \p loc to a RegularLocation.
@@ -627,10 +623,7 @@ class ReturnLocation : public SILLocation {
   /// Construct the return location for a constructor or a destructor.
   ReturnLocation(BraceStmt *BS);
 
-  ReturnLocation(FilenameAndLocation *filePos)
-    : SILLocation(filePos, ReturnKind) {}
-
-  ReturnLocation(FilenameAndLocation *filePos, bool Implicit)
+  ReturnLocation(FilenameAndLocation *filePos, bool Implicit = false)
     : SILLocation(filePos, ReturnKind, Implicit) {}
 
   static bool isKind(const SILLocation& L) {
@@ -651,10 +644,7 @@ class ImplicitReturnLocation : public SILLocation {
 
   ImplicitReturnLocation(AbstractFunctionDecl *AFD);
 
-  ImplicitReturnLocation(FilenameAndLocation *filePos)
-    : SILLocation(filePos, ImplicitReturnKind) {}
-
-  ImplicitReturnLocation(FilenameAndLocation *filePos, bool Implicit)
+  ImplicitReturnLocation(FilenameAndLocation *filePos, bool Implicit = false)
     : SILLocation(filePos, ImplicitReturnKind, Implicit) {}
 
   /// Convert \p loc to an ImplicitReturnLocation.

lib/Serialization/DeserializeSIL.cpp

@@ -396,8 +396,11 @@ SILDeserializer::readNextRecord(SmallVectorImpl<uint64_t> &scratch) {
 
 std::optional<SILLocation> SILDeserializer::readLoc(unsigned kind,
                                      SmallVectorImpl<uint64_t> &scratch) {
-  unsigned LocationKind = 0, Implicit;
+  unsigned LocationKind, Implicit = 0;
   SILLocation::FilenameAndLocation *FNameLoc = nullptr;
+  // Each SourceLoc opaque pointer is serialized once. Successive appearences
+  // are serialized as references to earlier serializations, with LocID as the
+  // indexing key
   if (kind == SIL_SOURCE_LOC_REF) {
     ValueID LocID;
     SourceLocRefLayout::readRecord(scratch, LocID, LocationKind, Implicit);
@@ -415,16 +418,16 @@ std::optional<SILLocation> SILDeserializer::readLoc(unsigned kind,
     ParsedLocs.push_back(FNameLoc);
   }
 
-  // FIXME: Instructions are being deserialized as implicit as existing code in
-  // readSILInstruction also deserializes instructions as implicit by default.
-  // This suppresses some diagnostics which otherwise break tests. This should
-  // be fixed in both places
   switch(LocationKind) {
     case SILLocation::ReturnKind:
       return ReturnLocation(FNameLoc, Implicit);
     case SILLocation::ImplicitReturnKind:
       return ImplicitReturnLocation(FNameLoc, Implicit);
-    default:
+    case SILLocation::InlinedKind:
+    case SILLocation::MandatoryInlinedKind:
+    case SILLocation::CleanupKind:
+    case SILLocation::ArtificialUnreachableKind:
+    case SILLocation::RegularKind:
       return RegularLocation(FNameLoc, Implicit);
   }
 }

lib/Serialization/SerializeSIL.cpp

@@ -3094,9 +3094,13 @@ void SILSerializer::writeSourceLoc(SILLocation Loc, const SourceManager &SM) {
     case SILLocation::ImplicitReturnKind:
       LocationKind = SILLocation::ImplicitReturnKind;
       break;
-    //TODO: handle other cases correctly
-    default:
+    case SILLocation::InlinedKind:
+    case SILLocation::MandatoryInlinedKind:
+    case SILLocation::CleanupKind:
+    case SILLocation::ArtificialUnreachableKind:
+    case SILLocation::RegularKind:
       LocationKind = SILLocation::RegularKind;
+      break;
   }
 
   if (SourceLocMap.find(OpaquePtr) != SourceLocMap.end()) {