[PATCH] D27546: [ASTReader] Sort RawComments before merging

2017-04-14 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

> 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


https://reviews.llvm.org/D27546



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27546: [ASTReader] Sort RawComments before merging

2017-04-13 Thread Richard Smith via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/Serialization/ASTReader.cpp:8487
+std::sort(Comments.begin(), Comments.end(),
+  BeforeThanCompare(SourceMgr));
 Context.Comments.addDeserializedComments(Comments);

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


https://reviews.llvm.org/D27546



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27546: [ASTReader] Sort RawComments before merging

2017-04-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno closed this revision.
bruno added a comment.

Done in r290134


https://reviews.llvm.org/D27546



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27546: [ASTReader] Sort RawComments before merging

2016-12-17 Thread Manman Ren via Phabricator via cfe-commits
manmanren accepted this revision.
manmanren added a comment.
This revision is now accepted and ready to land.

LGTM.

Manman


https://reviews.llvm.org/D27546



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27546: [ASTReader] Sort RawComments before merging

2016-12-12 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

Ping!


https://reviews.llvm.org/D27546



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27546: [ASTReader] Sort RawComments before merging

2016-12-07 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno created this revision.
bruno added reviewers: manmanren, akyrtzi, rsmith.
bruno added a subscriber: cfe-commits.

`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(SourceMgr)` or lambdas, and a 
`SourceMgr` is needed to perform the sort.


https://reviews.llvm.org/D27546

Files:
  lib/Serialization/ASTReader.cpp


Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -8481,6 +8481,10 @@
   }
 }
   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(SourceMgr));
 Context.Comments.addDeserializedComments(Comments);
   }
 }


Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -8481,6 +8481,10 @@
   }
 }
   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(SourceMgr));
 Context.Comments.addDeserializedComments(Comments);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits