malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.
================
Comment at: clangd/ClangdUnit.cpp:81
+ std::map<SourceLocation, std::string> takeIncludeMap() {
+ return std::move(IncludeMap);
----------------
takeIncludeLocationMap?
================
Comment at: clangd/ClangdUnit.cpp:105
+ const SourceManager &SM = CI.getSourceManager();
+ unsigned n = SM.local_sloc_entry_size();
+ SmallVector<SourceLocation, 10> InclusionStack;
----------------
n -> NumSlocs?
================
Comment at: clangd/ClangdUnit.cpp:107
+ SmallVector<SourceLocation, 10> InclusionStack;
+ std::map<SourceLocation, std::string>::iterator it = IncludeMap.begin();
+
----------------
do you need that iterator?
================
Comment at: clangd/ClangdUnit.cpp:109
+
+ for (unsigned i = 0; i < n; ++i) {
+ bool Invalid = false;
----------------
i -> I
================
Comment at: clangd/ClangdUnit.cpp:137
+ FI.getContentCache()->OrigEntry->tryGetRealPathName();
+ if (FilePath.empty()) {
+ // FIXME: Does tryGetRealPathName return empty if and only if the path
----------------
I think you can just skip it if empty (continue)
================
Comment at: clangd/ClangdUnit.cpp:143
+ }
+ IncludeMap.insert(std::pair<SourceLocation, std::string>(
+ InclusionStack.front(), FilePath.str()));
----------------
I think you can do instead
IncludeMap.insert({InclusionStack.front(), FilePath.str()});
================
Comment at: clangd/ClangdUnit.cpp:151
std::vector<serialization::DeclID> TopLevelDeclIDs;
+ std::map<SourceLocation, std::string> IncludeMap;
};
----------------
IncludeMap -> IncludeLocationMap ?
================
Comment at: clangd/ClangdUnit.cpp:800
- void addDeclarationLocation(const SourceRange &ValSourceRange) {
+ bool isSameLine(unsigned line) const {
+ const SourceManager &SourceMgr = AST.getSourceManager();
----------------
line -> Line
================
Comment at: clangd/ClangdUnit.cpp:806
+ void addDeclarationLocation(const SourceRange &ValSourceRange,
+ bool test = false) {
const SourceManager &SourceMgr = AST.getSourceManager();
----------------
remove bool test?
================
Comment at: clangd/ClangdUnit.cpp:824
+
+ void addLocation(URI uri, Range R) {
+
----------------
uri -> Uri
================
Comment at: clangd/ClangdUnit.cpp:834
+
+ for (auto it = IncludeMap.begin(); it != IncludeMap.end(); ++it) {
+ SourceLocation L = it->first;
----------------
it -> It
================
Comment at: clangd/ClangdUnit.cpp:837
+ std::string &Path = it->second;
+ Range r = Range();
+ unsigned line = AST.getSourceManager().getSpellingLineNumber(L);
----------------
r -> R
================
Comment at: clangd/ClangdUnit.cpp:839
+ unsigned line = AST.getSourceManager().getSpellingLineNumber(L);
+ if (isSameLine(line))
+ addLocation(URI::fromFile(Path), Range());
----------------
line -> Line
================
Comment at: clangd/ClangdUnit.h:136
std::vector<DiagWithFixIts> Diags;
+ std::map<SourceLocation, std::string> IncludeMap;
};
----------------
IncludeLocationMap?
================
Comment at: clangd/ClangdUnit.h:267
+ clangd::Logger &Logger,
+ std::map<SourceLocation, std::string>);
----------------
do you want to add a name to the parameter here?
================
Comment at: unittests/clangd/ClangdTests.cpp:902
-TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) {
+TEST_F(ClangdVFSTest, CheckDefinitionIncludes) {
MockFSProvider FS;
----------------
the test "CheckSourceHeaderSwitch" was removed
https://reviews.llvm.org/D38639
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
