a.sidorin added a comment.
That's excellent. Thank you for this work, Sean!
================
Comment at: tools/clang-import-test/CMakeLists.txt:19
+ )
\ No newline at end of file
----------------
Please add a newline here.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:106
+
+ const char *LineEnd = nullptr;
+
----------------
How about something like this:
```
StringRef Remain(LineBegin, Buffer->getBufferEnd() - LineBegin);
size_t EndPos = Remain.find_first_of("\r\n");
StringRef Line = (EndPos == StringRef::npos) ? Remain : StringRef(LineBegin,
EndPos);
llvm::errs() << Line << "\n";
llvm::errs().indent(LocColumn) << "^\n";
```
?
================
Comment at: tools/clang-import-test/clang-import-test.cpp:115
+
+ fprintf(stderr, "%s\n", LineString.c_str());
+ std::string Space(LocColumn, ' ');
----------------
Why do we use `fprintf` instead of `llvm::errs()`?
================
Comment at: tools/clang-import-test/clang-import-test.cpp:130
+
+ SmallVector<char, 16> DiagText;
+ Info.FormatDiagnostic(DiagText);
----------------
SmallString? So, you will not need to push_back.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:143
+ if (!Invalid) {
+ llvm::errs() << Ref.str().c_str() << '\n';
+ }
----------------
No need in `c_str()` here.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:152
+class TestExternalASTSource : public ExternalASTSource {
+private: llvm::ArrayRef<CompilerInstance *> ImportCIs;
+ std::map<CompilerInstance *, std::unique_ptr<ASTImporter>> ForwardImporters;
----------------
Please add a newline here.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:162
+ const bool MinimalImport = true;
+ ForwardImporters.emplace(std::make_pair(
+ ImportCI,
----------------
`ForwardImporters[ImportCI] = ...llvm::make_unique<>`?
================
Comment at: tools/clang-import-test/clang-import-test.cpp:179
+ DeclarationName Name) override {
+ std::vector<NamedDecl *> Decls;
+
----------------
SmallVector?
================
Comment at: tools/clang-import-test/clang-import-test.cpp:181
+
+ if (llvm::isa<TranslationUnitDecl>(DC)) {
+ for (CompilerInstance *I : ImportCIs) {
----------------
In LLVM code, qualifiers for llvm-style casts are not commonly used.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:222
+ SmallVectorImpl<Decl *> &Result) override {
+
+ }
----------------
Extra spaces.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:236
+ std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](std::string &s) -> const char * { return s.data(); });
+
----------------
`[](const std::string &s)`
================
Comment at: tools/clang-import-test/clang-import-test.cpp:238
+
+ CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.data(),
+ &ClangArgv.data()[ClangArgv.size()],
----------------
`ClangArgv.begin(), ClangArgv.end()`
================
Comment at: tools/clang-import-test/clang-import-test.cpp:327
+}
+}
+
----------------
// end namespace
Repository:
rL LLVM
https://reviews.llvm.org/D27180
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
