This is an archive of the discontinued LLVM Phabricator instance.

[clang][ExtractAPI] Create extractapi::RecordLocation
Needs ReviewPublic

Authored by Arsenic on Aug 13 2023, 2:27 AM.

Details

Reviewers
dang
ributzka
Summary

Create and use extractapi::RecordLocation instead of conventional
clang::PresumedLoc to track the location of an APIRecord, this reduces
the dependency of APISet on SourceManager and would help if someone
wants to create APISet from JSON Serialized SymbolGraph.

These changes also add extractapi::CommentLine which is similar to
RawComment::CommentLine but use RecordLocation instead of PresumedLoc.

Diff Detail

Event Timeline

Arsenic created this revision.Aug 13 2023, 2:27 AM
Herald added a project: Restricted Project. · View Herald Transcript
Arsenic requested review of this revision.Aug 13 2023, 2:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2023, 2:27 AM
Arsenic updated this revision to Diff 549691.Aug 13 2023, 2:57 AM

Fix minor typo in code comments

dang added inline comments.Aug 21 2023, 3:49 AM
clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
270–272

Can you refactor this code to construct the DocComment into it's own function?

Arsenic updated this revision to Diff 552794.EditedAug 23 2023, 10:46 AM
  • Create function to directly get DocComments from Decl
  • Update the way RecordLocation holds filename
  • Update changes to use RecordLocation with new C++ APIRecords
Arsenic marked an inline comment as done.Aug 23 2023, 10:49 AM
dang added a comment.Aug 24 2023, 12:51 AM

LGTM

clang/include/clang/ExtractAPI/API.h
137

s/suite/suit

Arsenic updated this revision to Diff 553730.EditedAug 26 2023, 7:06 AM
  • Move creation of DocComment in it's own separate function
  • Update the Underlying DataType used by RecordLocation
  • Update the patch to account for new C++ APIRecords
Arsenic marked an inline comment as done.Aug 26 2023, 7:07 AM
ributzka added inline comments.Sep 1 2023, 4:31 PM
clang/include/clang/ExtractAPI/API.h
141

There is an opportunity for optimization by avoiding the allocation of separate strings for each source location, especially since many source locations will be in the same file. As an example, APISet utilizes a BumpPtrAllocator to allocate and deduplicate strings. It is recommended to consider using the same allocator or a similar concept.

dang added a comment.Sep 6 2023, 7:02 AM

As per https://discourse.llvm.org/t/pull-request-migration-schedule/71595 we should move this review to GitHub to make sure we don't lose track of it.