[ASTReader] Sort RawComments before merging

[bruno]

Summary

RawComments are sorted by comparing underlying SourceLocation's. This is done by comparing FileID and Offset; when the FileID is the same it means
the locations are within the same TU and the Offset is used, etc.

FileID, from the source code: "A mostly-opaque identifier, where 0 is "invalid", >0 is this module, and <-1 is something loaded from another
module.". That said, when de-serializing SourceLocations, FileID's from RawComments loaded from other modules get negative IDs where previously they
were positive. This makes imported RawComments unsorted, leading to a wrong merge with other comments from the current TU. Fix that by sorting RawComments
properly after de-serialization and before merge.

This fixes an assertion in ASTContext::getRawCommentForDeclNoCache, which fires only in a debug build of clang. There's seems to be no reliable way to test this.
Additionally, I tried to use llvm::array_pod_sort, but that didn't seem to cope well with BeforeThanCompare<RawComment>(SourceMgr) or lambdas, and a SourceMgr is needed to perform the sort.

[bruno]

Ping!

[manmanren]

LGTM.

Manman

[bruno]

Done in r290134

[rsmith]

lib/Serialization/ASTReader.cpp

8487

Does this cause us to deserialize the SLocEntry for every FileID referenced by a RawComment? That would seem pretty bad.

[bruno]

Does this cause us to deserialize the SLocEntry for every FileID referenced by a RawComment? That would seem pretty bad.

I don't recall because It's been a while, but I'm gonna take a look and get back to you. Thanks

lib/Serialization/ASTReader.cpp

while (true) {
         Comments.push_back(new (Context) RawComment(
             SR, Kind, IsTrailingComment, IsAlmostTrailingComment,
             Context.getLangOpts().CommentOpts.ParseAllComments));
         break;
       }
       }
     }
   NextCursor:
+    // De-serialized SourceLocations get negative FileIDs for other modules,
+    // potentially invalidating the original order. Sort it again.
+    std::sort(Comments.begin(), Comments.end(),
+              BeforeThanCompare<RawComment>(SourceMgr));
     Context.Comments.addDeserializedComments(Comments);
   }
 }
 
 std::string ASTReader::getOwningModuleNameForDiagnostic(const Decl *D) {
   // If we know the owning module, use it.
   if (Module *M = D->getImportedOwningModule())
     return M->getFullModuleName();