This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Use our own relation kind.
ClosedPublic

Authored by hokein on Oct 15 2019, 4:09 AM.

Details

Summary

Move the RelationKind from Serialization.h to Relation.h. This patch doesn't
introduce any breaking changes.

Diff Detail

Event Timeline

hokein created this revision.Oct 15 2019, 4:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2019, 4:09 AM
Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-37

See http://jenkins.llvm-merge-guard.org/job/Phabricator/37/ for more details.

hokein updated this revision to Diff 225017.Oct 15 2019, 6:10 AM

update a style comment.

Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-38

See http://jenkins.llvm-merge-guard.org/job/Phabricator/38/ for more details.

kadircet added inline comments.Oct 15 2019, 6:36 AM
clang-tools-extra/clangd/index/MemIndex.h
72

can we rather use uint8_t in here instead of RelationKind to get rid of the DenseMapInfo specialization completely?

clang-tools-extra/clangd/index/Relation.h
22

is this clang-formatted ?

clang-tools-extra/clangd/index/Serialization.h
88

could you also delete definitions of these two functions?

hokein updated this revision to Diff 225034.Oct 15 2019, 7:26 AM
hokein marked 4 inline comments as done.

address comments.

Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2019, 7:26 AM
hokein added inline comments.Oct 15 2019, 7:27 AM
clang-tools-extra/clangd/index/MemIndex.h
72

Yes, done.

clang-tools-extra/clangd/index/Relation.h
22

yes, note that I'm leaving a trailing "," on purpose, to prevent clang-format the whole structure into one-line.

kadircet added inline comments.Oct 15 2019, 7:34 AM
clang-tools-extra/clangd/index/MemIndex.cpp
95 ↗(On Diff #225034)

static_cast<>

clang-tools-extra/clangd/index/MemIndex.h
74

"RelationKind should be of same size as a uint8_t"

llvm/include/llvm/ADT/DenseMapInfo.h
70 ↗(On Diff #225034)

s/unsign/unsigned/

74 ↗(On Diff #225034)

s/unsigned/unsigned char/
s/char/unsigned char/

76 ↗(On Diff #225034)

s/char/unsigned char/

hokein updated this revision to Diff 225037.Oct 15 2019, 7:48 AM
hokein marked 5 inline comments as done.

address comments.

hokein updated this revision to Diff 225039.Oct 15 2019, 7:50 AM

fix a missing static_cast.

hokein updated this revision to Diff 225041.Oct 15 2019, 7:51 AM

more static_cast

Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-40

See http://jenkins.llvm-merge-guard.org/job/Phabricator/40/ for more details.

kadircet accepted this revision.Oct 15 2019, 7:59 AM

Thanks, LGTM

This revision is now accepted and ready to land.Oct 15 2019, 7:59 AM
Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-41

See http://jenkins.llvm-merge-guard.org/job/Phabricator/41/ for more details.

Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-43

See http://jenkins.llvm-merge-guard.org/job/Phabricator/43/ for more details.

Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-42

See http://jenkins.llvm-merge-guard.org/job/Phabricator/42/ for more details.

This revision was automatically updated to reflect the committed changes.