Page MenuHomePhabricator

[ASTReader] Sort RawComments before merging
ClosedPublic

Authored by bruno on Dec 7 2016, 2:18 PM.

Details

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.

Diff Detail

Event Timeline

bruno updated this revision to Diff 80664.Dec 7 2016, 2:18 PM
bruno retitled this revision from to [ASTReader] Sort RawComments before merging.
bruno updated this object.
bruno added reviewers: manmanren, akyrtzi, rsmith.
bruno added a subscriber: cfe-commits.
manmanren accepted this revision.Dec 17 2016, 7:44 PM
manmanren edited edge metadata.

LGTM.

Manman

This revision is now accepted and ready to land.Dec 17 2016, 7:44 PM
bruno closed this revision.Apr 13 2017, 12:52 PM

Done in r290134

rsmith added inline comments.Apr 13 2017, 1:17 PM
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 added a comment.Apr 14 2017, 1:06 PM

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