This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Update the include mappings for std symbols.
Needs ReviewPublic

Authored by hokein on Jan 27 2020, 1:53 AM.

Details

Reviewers
kadircet
Summary

Use the latest offline version of cppreference.

Diff Detail

Event Timeline

hokein created this revision.Jan 27 2020, 1:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2020, 1:53 AM

Unit tests: pass. 62202 tests passed, 0 failed and 815 were skipped.

clang-tidy: fail. clang-tidy found 15 errors and 0 warnings. 15 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

we seem to be missing some symbols now, like polymorphic_allocator random_shuffle gets etc. and there seems to be a functional change to the parser. Are they intentional, if so why?

hokein updated this revision to Diff 240520.Jan 27 2020, 3:25 AM

refine the code to avoid confusion.

we seem to be missing some symbols now, like polymorphic_allocator random_shuffle gets etc.

yeap, two majoir reasons:

  • some symbols are deprecated/removed in new C++ standard, e.g. random_shuffle is removed in C++17
  • missing information in cppreference page, e.g. there is no header section for polymorphic_allocator in the offline cppreference page :(

Regarding those deprecated symbols, we could handle them according to which C++ standard we are using, as we have enough information from cppreferences, but it needs some work, and is low priority.

and there seems to be a functional change to the parser. Are they intentional, if so why?

The change should be NFC, it just makes the parser more robust on invalid links, the new version of the cppreference have invalid links on the symbol index page.
updated the code to make it clearer.

Unit tests: pass. 62202 tests passed, 0 failed and 815 were skipped.

clang-tidy: fail. clang-tidy found 15 errors and 0 warnings. 15 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

we seem to be missing some symbols now, like polymorphic_allocator random_shuffle gets etc.

yeap, two majoir reasons:

  • some symbols are deprecated/removed in new C++ standard, e.g. random_shuffle is removed in C++17
  • missing information in cppreference page, e.g. there is no header section for polymorphic_allocator in the offline cppreference page :(

Regarding those deprecated symbols, we could handle them according to which C++ standard we are using, as we have enough information from cppreferences, but it needs some work, and is low priority.

Ah I see, it is sad that cppreference just deletes those from symbol index, instead of marking them as delete after c++XX, I am not sure when the symbols introduced by this patch are added(but it is likely that they are introduced with c++20), so I think losing existing symbols
has a worse effect than not having information regarding c++20 symbols. Because these are symbols that are likely to be used by current c++ developers and supported by clangd, and it will be a regression to drop those, whereas the newly introduced symbols are already
missing.

So maybe postpone this update until we implement parsing of zombie_names page as well, also please file a bug either way so that we don't lose track.

and there seems to be a functional change to the parser. Are they intentional, if so why?

The change should be NFC, it just makes the parser more robust on invalid links, the new version of the cppreference have invalid links on the symbol index page.
updated the code to make it clearer.

maybe separate that out into a different patch so that it stands out?