Details
Diff Detail
- Repository
- rCTE Clang Tools Extra
- Build Status
Buildable 17917 Build 17917: arc lint + arc unit
Event Timeline
LGTM, just a few non-blocking NITs with questions.
clangd/SourceCode.cpp | ||
---|---|---|
180 | NIT: the comment about json array does not make any sense, given that we merely return a vector. | |
clangd/SourceCode.h | ||
70 | Do we really need to expose separate functions for vector<tooling::Replacement> and tooling::Replacements, given that both are trivial: loop over the array, convert each item? |
- Got rid of a helper for std::vector<Replacement>
clangd/SourceCode.h | ||
---|---|---|
70 | Good point. Use of std::vector<Replacement> should be discouraged anyway. I removed this function and inline the conversion for std::vector<Replacement> as this is only used in rename (which should probably be fixed to use tooling::Replacements instead). |
Do we really need to expose separate functions for vector<tooling::Replacement> and tooling::Replacements, given that both are trivial: loop over the array, convert each item?
Having them in .cpp file didn't hurt, but they seems redundant in the public header.
WDYT?