This is an archive of the discontinued LLVM Phabricator instance.

[include-fixer] make HeaderMapCollector maps from regex instead of postfix.
ClosedPublic

Authored by ioeric on Jun 28 2016, 1:45 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ioeric updated this revision to Diff 62071.Jun 28 2016, 1:45 AM
ioeric retitled this revision from to [include-fixer] make HeaderMapCollector maps from regex instead of postfix..
ioeric updated this object.
ioeric added a reviewer: bkramer.
ioeric added subscribers: hokein, cfe-commits.
bkramer added inline comments.Jul 4 2016, 1:42 AM
include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
16 ↗(On Diff #62071)

Can we make this a std::vector<std::pair<llvm::Regex, const char *>>? Creating llvm::Regex is somewhat expensive so we only want to do it once. We also never lookup into the postfix header map and only iterate over it so using a StringMap for it is wasteful.

ioeric updated this revision to Diff 62656.Jul 4 2016, 3:05 AM
ioeric marked an inline comment as done.
  • Change RegexHeaderMap from StringMap to std::vector<std::pair<StringRef, StringRef>>.
include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
16 ↗(On Diff #62071)

As per our offline discussion, copy constructor of llvm::Regex is deleted and the non-const match function prevents us from making the header map const. So we use std::vector<std::pair<StringRef, StringRef>> instead.

bkramer accepted this revision.Jul 4 2016, 3:31 AM
bkramer edited edge metadata.

If the regex construction turns out to be too slow we can fix it later. LGTM for now.

This revision is now accepted and ready to land.Jul 4 2016, 3:31 AM
This revision was automatically updated to reflect the committed changes.