This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Drop path suffix mapping for std symbols
Needs ReviewPublic

Authored by kadircet on Sep 24 2020, 2:06 AM.

Details

Reviewers
sammccall
Summary

As mentioned in D88144, this is only used as a fallback for symbols
missing in our stdsymbol mappings and preventing us from figuring out what's
broken.

After this patch we'll regress some cases, e.g. std::move, but those were
already broken for libc++ or C.


I am also happy with coming up with special casing sfor obvious regressions,
like std::move, before landing. The special casing I have in mind could either
be:

  • having a more targeted version of the suffix mapping targeted at those symbols, so we can stop the bleeding as we figure out cases even if we can't invest in a more principled solution.
  • just picking a single header in case of ambigious symbols, this might end up being annoying as clangd will insist on inserting the wrong header.
  • disable include insertion completely for problematic symbols until we figure out a solution.

Diff Detail

Event Timeline

kadircet created this revision.Sep 24 2020, 2:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 24 2020, 2:06 AM
kadircet requested review of this revision.Sep 24 2020, 2:06 AM
nridge added a subscriber: nridge.Sep 27 2020, 1:13 PM

Taking another look at the header list, there are a couple of classes of symbols beyond c/c++ standard library.

We don't have an alternative database for these. Maybe we should keep the mechanism but stop using it for the standard library? What do you think?

clang-tools-extra/clangd/index/CanonicalIncludes.h
46–47

should we update these comments?

51

hmm, are we going to regress all of posix?

52

and builtins, these are actually owned by us so should be portable

kadircet marked an inline comment as done.Oct 12 2020, 3:08 AM

We don't have an alternative database for these. Maybe we should keep the mechanism but stop using it for the standard library? What do you think?

Yes i think you are right. Just dropping STL mappings from the list.

kadircet updated this revision to Diff 297528.Oct 12 2020, 3:08 AM
  • Only drop STL mapping instead of getting rid of suffix mapping completely.