hokein added inline comments.
================
Comment at: include-fixer/IncludeFixer.cpp:64
getIncludeFixerContext(const clang::SourceManager &SourceManager,
clang::HeaderSearch &HeaderSearch) {
+ return SemaSource.getIncludeFixerContext(SourceManager, HeaderSearch);
----------------
also make it const?
================
Comment at: include-fixer/IncludeFixer.cpp:124
+ auto Reps = createIncludeFixerReplacements(Code, Context,
+ format::getLLVMStyle(), false);
+ if (!Reps)
----------------
nit: `/*AddQualifiers=*/false`.
================
Comment at: include-fixer/IncludeFixer.cpp:136
+
+ auto Begin = StartOfFile.getLocWithOffset(Placed.getOffset());
+ auto End = Begin.getLocWithOffset(Placed.getLength());
----------------
I have a concern that `Placed` here might be not the replacement for inserting
the new header, becuase the `Reps` returned from
`createIncludeFixerReplacements` may have some replacements for cleanup.
To make it more correct, maybe we can check whether
`Placed.getReplacementText()` is equal to `"#include" +
Context.getHeaderInfos().front().Header`?
================
Comment at: include-fixer/IncludeFixer.cpp:283
+ if (GenerateDiagnostics) {
+ FileID FID = CI->getSourceManager().getFileID(Typo.getLoc());
+ StringRef Code = CI->getSourceManager().getBufferData(FID);
----------------
You can use `SM` directly here since you have created a `SM` variable for
`SourceManager` above? The same below.
================
Comment at: include-fixer/IncludeFixer.h:119
+ IncludeFixerContext
+ getIncludeFixerContext(const clang::SourceManager &SourceManager,
+ clang::HeaderSearch &HeaderSearch);
----------------
Make it a `const` member function?
================
Comment at: include-fixer/IncludeFixer.h:124
+ /// Query the database for a given identifier.
+ bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range
Range);
+ CompilerInstance *CI;
----------------
A blank line below.
================
Comment at: include-fixer/plugin/IncludeFixerPlugin.cpp:1
+//===- ClangTidyPlugin.cpp - clang-tidy as a clang plugin
-----------------===//
+//
----------------
nit: IncludeFixerPlugin
================
Comment at: include-fixer/plugin/IncludeFixerPlugin.cpp:91
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the clang-tidy plugin.
+volatile int ClangIncludeFixerPluginAnchorSource = 0;
----------------
nit: clang-tidy=>include-fixer
https://reviews.llvm.org/D26752
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
