klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
================
Comment at: include-fixer/InMemoryXrefsDB.cpp:24-26
@@ +23,5 @@
+ for (const auto &Header : Entry.second) {
+ SymbolInfo Info;
+ Info.Name = Names.back();
+ Info.FilePath = Header;
+ for (auto IdentiferContext = Names.rbegin() + 1;
----------------
hokein wrote:
> Yeah, the intention is that it's not easy to construct a **completed**
> instance at one time since it has many fields.
Overall, I think creating an instance and then setting members should only be
done from places very strongly coupled to the class.
Generally, I think in these cases creating static members of the class that
construct the objects from the various data sources you have (like in this case
from a string, and a vector<string>) are preferable; not necessary to address
this in this CL, but please add a FIXME.
(also, specifically regarding the comment that it's not easy to construct a
completed instance: that is a large hint that it should be encoded somewhere
close to the class)
================
Comment at: include-fixer/IncludeFixer.h:34-35
@@ -33,3 +33,4 @@
IncludeFixerActionFactory(
- XrefsDB &Xrefs, std::vector<clang::tooling::Replacement> &Replacements,
+ XrefsDBManager &XrefsDBMgr,
+ std::vector<clang::tooling::Replacement> &Replacements,
bool MinimizeIncludePaths = true);
----------------
ioeric wrote:
> klimek wrote:
> > That seems unexpected. Why is the XrefsDBManager in the business of doing
> > SymbolInfo -> include path mappings?
> Since the mapping was done in XrefsDB before, I kept the way it was.
> Migrating SymbolInfo -> Include path mappings into IncludeFixer will be the
> next step, and I'll need to discuss with Ben about where exactly we want to
> place it.
Please add a FIXME that this is going to change.
================
Comment at: unittests/include-fixer/IncludeFixerTest.cpp:86
@@ +85,3 @@
+ // string without "std::" can also be fixed since fixed db results go through
+ // XrefsDBManager, and XrefsDBManager matches unqualified identifier too.
+ EXPECT_EQ("#include <string>\nstring foo;\n",
----------------
identifiers
http://reviews.llvm.org/D19869
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
