This is an archive of the discontinued LLVM Phabricator instance.

[find-all-symbols] Index partial template specializations.
ClosedPublic

Authored by hokein on Dec 19 2016, 8:32 AM.

Details

Summary

Fix no std::function index.

Previously, we don't index all the template specialization declarations of functions and classes (Because we assume that template functions/classes are indexed by their template declarations), but this is not always true in some cases like std::function which only has a partial template specialization declaration.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein updated this revision to Diff 81954.Dec 19 2016, 8:32 AM
hokein retitled this revision from to [find-all-symbols] Index partial template specializations..
hokein updated this object.
hokein added a reviewer: ioeric.
hokein added subscribers: bkramer, cfe-commits.
ioeric edited edge metadata.Dec 21 2016, 1:28 AM

Why do we allow partially specialization but fully specialization now? Could you elaborate in the cl description?

hokein updated this object.Dec 21 2016, 4:42 AM
hokein edited edge metadata.

Why do we allow partially specialization but fully specialization now? Could you elaborate in the cl description?

Done.

I'm not quite convinced/sure if we want to match all partial specializations just for std::function. We filtered out template specialization because there could be multiple specializations for a template in different headers and it was not clear which one we would choose. And if we do index specializations, I don't see why we would want to filter out full specializations.

We might want to wait for Ben's opinion.

ioeric removed a subscriber: bkramer.
bkramer accepted this revision.Jan 11 2017, 2:37 AM
bkramer edited edge metadata.

The reason why we disallowed specializations is that there are type-trait classes that get specialized all over the place (DenseMapInfo in LLVM is one example). I think we can get away with allowing partial specializations though. Those should be much less common and we can give the user a choice between the headers that contain them.

include-fixer/find-all-symbols/FindAllSymbols.cpp
39 ↗(On Diff #81954)

PascalCase for variable names.

This revision is now accepted and ready to land.Jan 11 2017, 2:37 AM
hokein updated this revision to Diff 83944.Jan 11 2017, 3:36 AM
hokein marked an inline comment as done.
hokein edited edge metadata.

Fix code style nit.

This revision was automatically updated to reflect the committed changes.