hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Looks good.
Comment at: lib/Basic/DiagnosticIDs.cpp:46
unsigned WarnShowInSystemHeader : 1;
- unsigned Category : 5;
+ unsigned Category : 6;
arphaman
hokein added inline comments.
Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:74
+/// An AST selection value that corresponds to a selection of a set of
+/// statements that belong to one body of code (like one function).
+///
I see all your tests ar
hokein added a comment.
This is a good start, a few comments. Also I'd suggest running clang-format on
this patch -- I saw a few places violate the code style.
Comment at: include/clang/Tooling/CommonOptionsParser.h:90
///
- /// This constructor exits program in case of er
hokein created this revision.
Herald added subscribers: mgorny, klimek.
Also contain some fixes:
- Get rid of the "TranslationUnitDecl" special case in RenameClass, as
we switch to use USR instead of AST matcher to find symbols. A bonus
point is that now the test cases make more sense.
- Fix a
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D38893
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
hokein updated this revision to Diff 119121.
hokein added a comment.
Update, and add one unfixed testcase.
https://reviews.llvm.org/D38882
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/CMakeLists.txt
unittests/Rename/RenameClassTest.cpp
unittests/Rename/RenameF
hokein added a comment.
Sorry, I thought I have run all the tests. The previous version broke one test
in RenameClass. We still need the workaround for the `std::function`
special case. I added it back, and also added a corresponding test case for
renaming function, but it is currently not su
hokein updated this revision to Diff 119128.
hokein marked an inline comment as done.
hokein added a comment.
Remove the FIXME.
https://reviews.llvm.org/D38882
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/CMakeLists.txt
unittests/Rename/RenameClassTest.cpp
uni
hokein added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:469
+ // name.
+ // FIXME: Consider using using-decls to shorten the namespace
+ // qualifers.
ioeric wrote:
> The current behavior looks fine to
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315898: [clang-rename] Add function unit tests. (authored by
hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D38882
Files:
cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
cfe/trunk
hokein created this revision.
Herald added subscribers: mgorny, klimek.
- Add unit tests for renaming enum.
- Support unscoped enum constants in expressions.
https://reviews.llvm.org/D38989
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/CMakeLists.txt
unittests/Re
hokein updated this revision to Diff 119274.
hokein marked 2 inline comments as done.
hokein added a comment.
address review comments, and add FIXME
https://reviews.llvm.org/D38989
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/CMakeLists.txt
unittests/Rename/Rena
hokein added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:212
+ // Ignore the case where there is no prefix qualifer for the enum
constant
+ // expression like `a = Green`.
+ if (!Expr->hasQualifier())
ioeric wrote:
hokein added a comment.
+sammccall who is currently working on clangD, he might have ideas on the
clangD/refactor integration.
Repository:
rL LLVM
https://reviews.llvm.org/D38985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lis
hokein updated this revision to Diff 119309.
hokein added a comment.
Use getQualifierLoc.
https://reviews.llvm.org/D38989
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/CMakeLists.txt
unittests/Rename/RenameEnumTest.cpp
Index: unittests/Rename/RenameEnumTest.cpp
hokein added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:212
+ // Ignore the case where there is no prefix qualifer for the enum
constant
+ // expression like `a = Green`.
+ if (!Expr->hasQualifier())
ioeric wrote:
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: unittests/Tooling/ASTSelectionTest.cpp:688
+ Source, {2, 2}, None,
+ [](SourceRange SelectionRange, Optional Node) {
+EXPECT_TRUE(Node);
-
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315999: [clang-rename] Rename enum. (authored by hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D38989
Files:
cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
cfe/trunk/unittests/Re
hokein created this revision.
Herald added subscribers: mgorny, klimek.
- Support rename alias.
- Add unittests for renaming alias.
- Don't generate fixes for the SourceLocations that are invalid or in temporary
buffer, otherwise crash would be happened when generating AtomicChanges.
https://re
hokein added inline comments.
Comment at: include/clang/Tooling/CommonOptionsParser.h:95
+ static llvm::Expected>
+ create(int &argc, const char **argv, llvm::cl::OptionCategory &Category,
worth some documentation?
Comment at: include/clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316074: [clang-rename] Rename alias. (authored by hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D39043
Files:
cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
cfe/trunk/unittests/R
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Looks good from my side. You might want @klimek to take a look before
committing.
Comment at: include/clang/Tooling/CommonOptionsParser.h:90
- ///
- /// This constructor e
hokein added a comment.
Awesome, thanks for the very well-illustrated tutorial!
Comment at: docs/RefactoringActionTutorial.rst:7
+
+ This tutorial talks about a work-in-progress library in Clang.
+ Some of the described features might not be available yet in trunk, but
shoul
hokein created this revision.
Change clang-refactor default behavior to print the new code after refactoring
(instead of editing the source files), which would make it easier to use
and debug the refactoring action.
https://reviews.llvm.org/D39092
Files:
test/Refactor/tool-apply-replacements.
hokein accepted this revision.
hokein added a comment.
Still LGTM.
https://reviews.llvm.org/D39042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein created this revision.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D39120
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/RenameFunctionTest.cpp
Index: unittests/Rename/RenameFunctionTest.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316212: [clang-refactor] Add "-Inplace" option to the
commandline tool. (authored by hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D39092
Files:
cfe/trunk/test/Refactor/tool-apply-replacemen
hokein added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:227
+SourceLocation EndLoc = Expr->hasExplicitTemplateArgs()
+? Expr->getLAngleLoc().getLocWithOffset(-1)
+: Expr->getLocE
hokein updated this revision to Diff 119654.
hokein marked an inline comment as done.
hokein added a comment.
Test for `foo ()`.
https://reviews.llvm.org/D39120
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/RenameFunctionTest.cpp
Index: unittests/Rename/RenameFu
hokein added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:227
+SourceLocation EndLoc = Expr->hasExplicitTemplateArgs()
+? Expr->getLAngleLoc().getLocWithOffset(-1)
+: Expr->getLocE
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316314: [rename] Don't overwrite the template argument when
renaming a template… (authored by hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D39120
Files:
cfe/trunk/lib/Tooling/Refactoring/Re
hokein added inline comments.
Comment at: unittests/Rename/RenameFunctionTest.cpp:232
+ std::string Expected = R"(
+ namespace na {
+ template T Y();
cierpuchaw wrote:
> Shouldn't this be `namespace nb {`?
It is intended. We don't change the namespace
hokein created this revision.
Herald added subscribers: mgorny, klimek.
https://reviews.llvm.org/D39178
Files:
lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
unittests/Rename/CMakeLists.txt
unittests/Rename/RenameMemberTest.cpp
Index: unittests/Rename/RenameMemberTest.cpp
hokein created this revision.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D39241
Files:
lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
test/clang-rename/TemplatedClassFunction.cpp
Index: test/clang-rename/TemplatedClassFunction.cpp
==
hokein updated this revision to Diff 120199.
hokein added a comment.
improve the code and add more tests.
https://reviews.llvm.org/D39241
Files:
lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
test/clang-rename/TemplatedClassFunction.cpp
Index: test/clang-rename/TemplatedClassFunction
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316561: [clang-rename] Fix and enable the failing
TemplatedClassFunction test. (authored by hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D39241
Files:
cfe/trunk/lib/Tooling/Refactoring/Rena
hokein updated this revision to Diff 120216.
hokein marked 2 inline comments as done.
hokein added a comment.
- remove the code of handling template class methods as it is fixed in another
patch.
- address review comments.
https://reviews.llvm.org/D39178
Files:
lib/Tooling/Refactoring/Rename
hokein added a comment.
I have refined the patch based on the https://reviews.llvm.org/D39241. Please
take another look.
https://reviews.llvm.org/D39178
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316571: [rename] support renaming class member. (authored by
hokein).
Repository:
rL LLVM
https://reviews.llvm.org/D39178
Files:
cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
cfe/trunk
hokein created this revision.
Herald added a subscriber: klimek.
This is first step to integrate qualified rename into clang-refactor.
Also a few changes to SymbolOccurrence:
- add more information to SymbolOccurrence
- remove the way of using SourceLocation as the array size
https://reviews.l
hokein added inline comments.
Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:17
+
+ absl::strings_internal::foo();
+ class foo {
I think this line is also triggered the warning?
Comment at: test/clang-tidy/abseil-no-internal-d
hokein updated this revision to Diff 162611.
hokein marked 7 inline comments as done.
hokein added a comment.
Update the patch based on our new discussion
- SymbolOccurrenceSlab for storing underlying occurrence data
- reuse SymbolCollector to collect symbol occurrences
Repository:
rCTE Clang
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
Implement the interface in
- FileIndex
- MemIndex
- MergeIndex
Depends on https://reviews.llvm.org/D50385.
Repository:
rCTE Clang Tools Ex
hokein added a comment.
Some numbers of memory usage from running this on my machine:
| File | Preamble AST | Main AST |
| SemaDecl.cpp | 14.5MB | 108.1KB (symbols) + 16.5KB (occurrences) |
| CodeComplete.cpp | 15.4MB | 53.9KB (symbols)
hokein added a comment.
Thanks for the comments.
In https://reviews.llvm.org/D50958#1212221, @sammccall wrote:
> I do have some questions there that would be good to discuss. Meanwhile,
> @hokein can you rebase this patch against head?
Yes, I'd love to, but since this patch is quite large, I
hokein added a comment.
In https://reviews.llvm.org/D51279#1214268, @ilya-biryukov wrote:
> Just noticed I'm not on the reviewers list, sorry for chiming in without
> invitation. Was really excited about the change :-)
Comments are always welcome :)
Comment at: clangd/index
hokein closed this revision.
hokein added a comment.
I have committed the patch for you, https://reviews.llvm.org/rL340800.
Comment at: test/clang-tidy/abseil-no-namespace.cpp:10
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:11: warning: namespace 'absl' i
hokein added a comment.
Please rebase your patch, since the absl matcher patch is submitted. Thanks.
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:8
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:1:23: warning: do not reference any 'internal'
namesp
hokein updated this revision to Diff 162854.
hokein marked 10 inline comments as done.
hokein added a comment.
Address review comments and fix code style.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/
hokein updated this revision to Diff 162856.
hokein added a comment.
Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd
hokein added inline comments.
Comment at: clangd/index/Index.h:377
+ llvm::ArrayRef find(const SymbolID &ID) const {
+ auto It = Occurrences.find(ID);
+ if (It == Occurrences.end())
sammccall wrote:
> return Occurrences.lookup(ID)?
The `DenseMap::lookup
hokein added inline comments.
Comment at: test/clang-tidy/Inputs/absl/external-file.h:1
+void DirectAcess2() { absl::strings_internal::InternalFunction(); }
+
The file can not self-compile, and we should make it compilable.
And put the newly-added code at the en
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM with a nit, thanks for your work.
Comment at: clang-tidy/abseil/NoInternalDependenciesCheck.h:20
+/// Finds instances where the user depends on internal details and warn
hokein updated this revision to Diff 163064.
hokein marked 16 inline comments as done.
hokein added a comment.
Herald added a subscriber: mgrang.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/Fil
hokein updated this revision to Diff 163066.
hokein added a comment.
Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
hokein added a comment.
Thanks for the comments.
Comment at: clangd/index/FileIndex.cpp:48
+ if (TopLevelDecls) { // index main AST, set occurrence flag.
+CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
ilya-biryukov wrote:
> There i
hokein added a comment.
Thanks very much for improving it!
Comment at: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h:45
- llvm::Regex RE(
- "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
- "synchronization|time|types|utility)");
---
hokein updated this revision to Diff 163284.
hokein marked 8 inline comments as done.
hokein added a comment.
- address review comments
- rescope the patch only focus on `findReferences` implementation.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
Files:
clangd/XRefs.
hokein added inline comments.
Comment at: clangd/XRefs.cpp:666
+std::vector references(ParsedAST &AST, Position Pos,
+ bool IncludeDeclaration,
+ const SymbolIndex *Index) {
sammccall wrote:
> I'm no
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D51360#1219024, @kbobyrev wrote:
> Ah, correct. I totally forgot about the `^string$` for the exact match.
>
> This should not change behavior now. I believe what yo
hokein added a comment.
Sorry, I just realized I didn't reply the comments in the first review (they
were in my draft).
Comment at: clangd/XRefs.cpp:71
+struct DeclInfo {
+ const Decl *D;
ilya-biryukov wrote:
> NIT: maybe call `Occurence` instead? As this i
hokein updated this revision to Diff 163311.
hokein marked 3 inline comments as done.
hokein added a comment.
Address review comments, return deterministic results.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM, note that this change will break our next internal integration.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51504
___
c
hokein closed this revision.
hokein added a comment.
Thanks @aaron.ballman. I'm closing it now as it is committed in
https://reviews.llvm.org/rL340915.
https://reviews.llvm.org/D51061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
hokein updated this revision to Diff 163512.
hokein added a comment.
Address review comments in https://reviews.llvm.org/D51279.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clan
hokein marked 3 inline comments as done.
hokein added a comment.
Sorry! Just realised I messed up this patch with
https://reviews.llvm.org/D50385 (mostly SymbolCollector changes), all the
comments about `SymbolCollector` are fixed in https://reviews.llvm.org/D50385.
Repository:
rCTE Clang To
hokein closed this revision.
hokein added a comment.
Committed in https://reviews.llvm.org/rL341208.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
hokein updated this revision to Diff 163531.
hokein marked 4 inline comments as done.
hokein added a comment.
- rebase
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/In
hokein updated this revision to Diff 163532.
hokein added a comment.
Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/MemIndex.h
hokein added inline comments.
Comment at: clangd/index/MemIndex.cpp:35
std::unique_ptr MemIndex::build(SymbolSlab Slab) {
auto Idx = llvm::make_unique();
sammccall wrote:
> This is still implicitly creating an index with no occurrences. Did you mean
> to a
hokein updated this revision to Diff 163539.
hokein marked 3 inline comments as done.
hokein added a comment.
address review comments:
- build memindex with symbol slab and occurrence slab
- remove withAllCode in TestTU
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51279
File
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341242: [clangd] Implement findOccurrences interface in
dynamic index. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51279?vs=163539&id=163580#toc
Repository
hokein updated this revision to Diff 163654.
hokein edited the summary of this revision.
hokein added a comment.
Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
Files:
clangd/XRefs.cpp
clangd/XRefs.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRe
hokein added a comment.
Nice! looks mostly good to me.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+ functionDecl(
+ isDefinition(),
+ unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
any reason why we restri
hokein added a comment.
Thanks for cleaning it up! I admit that `SymbolOccurrences` is a loong name. A
few nits.
Comment at: clangd/index/FileIndex.cpp:120
+ auto &SymRefs = Sym.second;
+ std::sort(SymRefs.begin(), SymRefs.end());
+ std::copy(SymRefs.begin(), Sy
hokein accepted this revision.
hokein added a comment.
LGTM, thanks!
Comment at: clangd/index/FileIndex.cpp:120
+ auto &SymRefs = Sym.second;
+ std::sort(SymRefs.begin(), SymRefs.end());
+ std::copy(SymRefs.begin(), SymRefs.end(), back_inserter(RefsStorage));
---
hokein added a comment.
In https://reviews.llvm.org/D50958#149, @sammccall wrote:
> Looking forward to using this!
>
> Unless you object, I'd like to address the remaining comments (and get a
> review), so you can make the most of your leave!
Thanks, feel free to pick it up. The comments m
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, looks good from my side.
Comment at: clangd/XRefs.cpp:328
+ : AST(AST) {
+for (const Decl *D : TargetDecls)
+ Targets.insert(D);
nit: w
hokein added inline comments.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+ functionDecl(
+ isDefinition(),
+ unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
stephanemoore wrote:
> hokein wrote:
> > any reason
hokein added a comment.
In https://reviews.llvm.org/D50438#1224287, @ilya-biryukov wrote:
> @hokein, would it be fine if I submit this change for you?
> It would be nice to get this fix in before you get back from vacation.
Thanks, and sorry for the delay here, I was planning to submit it afte
hokein updated this revision to Diff 164013.
hokein added a comment.
Rebase to master.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
==
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM, ship it!
Comment at: clangd/Protocol.h:881
+struct ReferenceParams : public TextDocumentPositionParams {};
+bool fromJSON(const llvm::json::Value &, Reference
hokein updated this revision to Diff 164014.
hokein added a comment.
Fix code style.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341463: [clangd] Sort GoToDefinition results. (authored by
hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50438
Files:
clang-tools-ext
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341463: [clangd] Sort GoToDefinition results. (authored by
hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50438?vs=164014&id=164017#toc
Repository:
rL LLVM
https://revi
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
We need to update the documentation
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html as
well.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D5181
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.
The implicit bool conversion could happen superisingly, e.g. when
checking `if (Loc1 == Loc2)`, the compiler will convert SymbolLocation to
bool before com
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.
This is the first step of implementing Xrefs in clangd:
- add index interfaces, and related data structures.
- implement a naive in-memory index for symbo
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: test/clang-tidy/fuchsia-multiple-inheritance-crash.cpp:1
+// RUN: clang-tidy -checks='fuchsia-multiple-inheritance' %s --
+template
nit: i
hokein added a comment.
Mostly good. A few nits.
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:103
// Do not trigger on non-const value parameters when they are not only used
as
// const.
This comment needs to be updated.
hokein marked an inline comment as done.
hokein added inline comments.
Comment at: clangd/index/Index.h:45
- operator bool() const { return !FileURI.empty(); }
+ explicit operator bool() const { return !FileURI.empty(); }
+ bool operator==(const SymbolLocation& Loc) const {
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rCTE338517: [clangd] Make SymbolLocation => bool conversion
explicitly. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D496
hokein updated this revision to Diff 158507.
hokein marked 4 inline comments as done.
hokein added a comment.
Scope the patch to interfaces only.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49658
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/I
hokein added a comment.
In https://reviews.llvm.org/D49658#1171410, @sammccall wrote:
> Mostly LG.
>
> I think we can simplify in a couple of places, and you should decide if this
> patch is *implementing* the new index operation or not. (Currently it
> implements it for 1/3 implementations, wh
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:108
return;
+ if (const auto *Ctor = dyn_cast(Function)) {
+for (const auto *Init : Ctor->in
hokein added a comment.
nit: The patch description needs to be updated.
https://reviews.llvm.org/D50154
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM.
Repository:
rC Clang
https://reviews.llvm.org/D50307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.o
hokein marked 2 inline comments as done.
hokein added inline comments.
Comment at: clangd/index/Index.h:288
+ Unknown = 0,
+ Declaration = static_cast(index::SymbolRole::Declaration),
+ Definition = static_cast(index::SymbolRole::Definition),
ilya-biryukov wro
hokein updated this revision to Diff 159287.
hokein marked 2 inline comments as done.
hokein added a comment.
Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49658
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.h
clangd/index/MemInde
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339011: [clangd] Index Interfaces for Xrefs (authored by
hokein, committed by ).
Herald added a subscriber: llvm-commits.
101 - 200 of 4290 matches
Mail list logo
