hokein updated this revision to Diff 128021.
hokein marked an inline comment as done.
hokein added a comment.
Use GeneralCateogry.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41491
Files:
clangd/CMakeLists.txt
clangd/global-symbol-builder/CMakeLists.txt
This revision was automatically updated to reflect the committed changes.
Closed by commit rL321358: [clangd] Add a tool to build YAML-format global
symbols. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41491?vs=128021=128022#toc
Repository:
rL
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Nice, LGTM.
Comment at: clangd/index/Index.h:108
//
// FIXME: Use a space-efficient implementation, a lot of Symbol fields could
// share the same storage.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: ilya-biryukov, mgorny, klimek.
The tools is used to generate global symbols for clangd (global code
completion),
The format is YAML, which is only for **experiment**.
TEST: used the tool to generate
hokein added inline comments.
Comment at: clangd/XRefs.cpp:68
+ // declaration, and it could be a forward declaration.
+ auto Def = std::find_if(D->redecls_begin(), D->redecls_end(),
+ [](const Decl *D) { return IsDefinition(D); });
hokein updated this revision to Diff 128603.
hokein marked an inline comment as done.
hokein added a comment.
Add index source information to the completion item.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41668
Files:
clangd/ClangdLSPServer.cpp
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 :
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:
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 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
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:
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,
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
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Looks good with a few nits. Looks like you didn't update the patch correctly.
You have marked comments done, but your code doesn't get changed accordingly.
Please double check with it.
I
hokein added a comment.
Sorry for the delay. I thought there was a dependent patch of this patch, but
it seems resolved, right?
A rough round of review.
> New version. I tried to share the code between this and SymbolCollector, but
> didn't find a good clean way. Do you have concrete
hokein added a comment.
In https://reviews.llvm.org/D50385#1191914, @ioeric wrote:
> 2 high-level questions:
>
> 1. What's the reason for having a separate `SymbolOccurrenceSlab`? Could
> store occurrences as extra payload of `Symbol`?
Storing occurrences in `Symbol` structure is easy to
hokein updated this revision to Diff 159919.
hokein marked an inline comment as done.
hokein added a comment.
Adress review comment - ignore the case in the check.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50447
Files:
clang-tidy/performance/ForRangeCopyCheck.cpp
hokein added inline comments.
Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:39
+ auto WhitelistClassMatcher =
+ cxxRecordDecl(hasAnyName(SmallVector(
+ WhitelistClasses.begin(), WhitelistClasses.end(;
JonasToth wrote:
> I have seens
hokein created this revision.
hokein added reviewers: ilya-biryukov, alexfh.
Herald added a subscriber: xazax.hun.
The upstream change r336737 causes a regression issue in our internal
base codebase.
Adding an option to the check allowing us whitelist these classes.
Repository:
rCTE Clang
hokein added a comment.
In https://reviews.llvm.org/D50447#1192280, @JonasToth wrote:
> If whitelisting works, thats fine. But I agree with @lebedev.ri that a
> contribution/improvement to the ExprMutAnalyzer is the better option. This is
> especially the case, because multiple checks (and
hokein added a comment.
> That looks remarkably like google benchmark main loop. (i don't know at hand
> if making this const will break it, but probably not?)
> I wonder if instead there should be an option to not complain about the
> variables that aren't actually used?
Yeah, it is google
hokein accepted this revision.
hokein added a comment.
looks good, a few nits.
Comment at: clangd/CodeComplete.cpp:742
+llvm::DenseMap FetchedDocs;
+if (Index) {
+ LookupRequest IndexRequest;
nit: do we want to log anything here? It may be useful
hokein updated this revision to Diff 160983.
hokein marked 4 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50695
Files:
clangd/TUScheduler.cpp
unittests/clangd/XRefsTests.cpp
Index:
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
looks good.
Comment at: clangd/CodeComplete.cpp:721
+ // would get 'std::basic_string').
+ if (auto Func = Candidate.getFunction()) {
+if (auto Pattern =
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, javed.absar.
Fix an inconsistent behavior of using `LastBuiltPreamble`/`NewPreamble`
in TUScheduler (see the test for details), AST should always use
NewPreamble.
hokein added a comment.
Looks mostly good.
Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration
objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1,
hokein updated this revision to Diff 160544.
hokein marked 3 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index:
hokein added a comment.
In https://reviews.llvm.org/D50627#1196988, @ilya-biryukov wrote:
> Maybe also add a test for find-definition that was broken before? (non-empty
> preamble -> empty preamble -> bad gotodef that goes to included file instead
> of the local variable)
> To have a
hokein added inline comments.
Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191
+void templated_thrower() { throw T{}(); }
+// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type
'int' is not derived from 'std::exception'
+
hokein added a comment.
> Hmm, I think this is the same for other symbol payload e.g. definition can be
> missing for a symbol. And it seems to me that the concern is on the
> SymbolSlab level: if a slab is for a single TU, users should expect missing
> information; if a slab is merged from
hokein added a comment.
@hugoeg, it looks like you are waiting for review, but this patch doesn't
include the `IsExpansionInAbseilHeader` matcher. What's your plan of it?
Comment at: test/clang-tidy/abseil-fake-declarations.h:1
+namespace std {
+struct string {
hokein updated this revision to Diff 161000.
hokein marked 2 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50627
Files:
unittests/clangd/TUSchedulerTests.cpp
Index:
hokein added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
JonasToth wrote:
> hugoeg wrote:
> > deannagarcia wrote:
> > >
hokein updated this revision to Diff 161176.
hokein marked an inline comment as done.
hokein added a comment.
Remove ASSERT_TRUE(bool(Preamble)).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50627
Files:
unittests/clangd/TUSchedulerTests.cpp
Index:
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE340001: [clangd] Always use the latest preamble (authored
by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50695?vs=160983=161177#toc
Repository:
rCTE Clang Tools
hokein added inline comments.
Comment at: clangd/index/Index.h:65
public:
+ static llvm::Optional forDecl(const Decl );
+
ilya-biryukov wrote:
> hokein wrote:
> > We already have this similar function in clangd/AST.h.
> Thanks for pointing this out.
> It's
hokein added inline comments.
Comment at: clangd/CodeComplete.cpp:755
+ });
+ log("SigHelp: requested docs for {0} symbols from the index, got {1} "
+ "symbols with non-empty docs in the response",
ioeric wrote:
> ilya-biryukov wrote:
> >
hokein added a comment.
Thanks for adding it.
Comment at: clangd/TUScheduler.cpp:406
// We only need to build the AST if diagnostics were requested.
if (WantDiags == WantDiagnostics::No)
return;
The AST might not get built if `WantDiags::No`,
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.
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
hokein created this revision.
hokein added reviewers: ilya-biryukov, ioeric.
Herald added subscribers: arphaman, mgrang, jkorous, MaskRay, mgorny.
This patch implements a SymbolOccurenceCollector, which will be used to:
- Find all occurrences in AST
- Find all occurrences in MemIndex
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339116: [clangd] Share getSymbolID implementation. (authored
by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50375
Files:
hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: arphaman, jkorous, MaskRay, ilya-biryukov.
And remove all duplicated implementation.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50375
Files:
clangd/AST.cpp
clangd/AST.h
hokein added inline comments.
Comment at: clangd/CodeComplete.cpp:399
case CodeCompletionResult::RK_Pattern: {
-llvm::SmallString<128> USR;
-if (/*Ignore=*/clang::index::generateUSRForDecl(R.Declaration, USR))
- return None;
-return SymbolID(USR);
+return
hokein updated this revision to Diff 159466.
hokein marked 3 inline comments as done.
hokein added a comment.
Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50375
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CodeComplete.cpp
clangd/XRefs.cpp
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
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, javed.absar.
clangd maintains the last good preamble for each TU and clang treats an empty
preamble as an error, therefore, clangd will use the stale preamble for
a
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Empty preamble is valid for source file which doesn't have any
preprocessor and #includes.
This patch makes clang treat an empty preamble as a normal preamble.
Check: ninja check-clang
A testcase is added in
hokein added inline comments.
Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:6
+
+Gives a warning if code using Abseil depends on internal details. If something
is in a namespace or filename/path that includes the word “internal”, code is
not allowed to depend
hokein added inline comments.
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:24
+
+ const auto IsDuration =
+ expr(hasType(cxxRecordDecl(hasName("::absl::Duration";
maybe call it `DurationExpr` since you have declared the variable as
hokein added a comment.
The check is missing its document, please add one in `docs/clang-tidy/checks/`.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
hokein added a comment.
Hello, this patch seems introduce a new crash, and I have reverted it in
r339558.
Here is the minimal test case:
class SCOPED_LOCKABLE FileLock {
public:
explicit FileLock()
EXCLUSIVE_LOCK_FUNCTION(file_);
~FileLock() UNLOCK_FUNCTION(file_);
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339415: [clang-tidy] Omit cases where loop variable is not
used in loop body in (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
hokein added a comment.
Thanks for contributing to clang-tidy!
Comment at: docs/clang-tidy/checks/abseil-duration-division.rst:3
+
+abseil-duration-division
+
This is a nice document. Does abseil have similar thing in its official
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: arphaman, mgrang, jkorous, MaskRay, ioeric.
GoToDefinition returns all declaration results (implicit/explicit) that are
in the same location, and the results are returned in arbitrary order.
Some LSP
hokein added inline comments.
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+ if (loc.isInvalid()) {
hugoeg wrote:
> hokein wrote:
> > I think we can make it as an ASTMatcher
hokein added a subscriber: sbenza.
hokein added a comment.
Thanks for porting the check to upstream (context: the check was developed
internally, and has been run on our codebase for a while, it is pretty stable).
Could you please update the patch message to indicate this is a porting change,
hokein added inline comments.
Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:20
+
+bool IsInAbseilFile(const SourceManager& manager, SourceLocation loc){
+ if (loc.isInvalid()) {
I think we can make it as an ASTMatcher instead of a function like:
```
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340035: [clangd] Add a testcase for empty preamble.
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50627
Files:
hokein closed this revision.
hokein added a comment.
Committed in https://reviews.llvm.org/rL340038.
https://reviews.llvm.org/D50389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein created this revision.
hokein added reviewers: ilya-biryukov, ioeric.
Herald added subscribers: arphaman, jkorous, MaskRay.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50896
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340029: [Preamble] Empty preamble is not an error. (authored
by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50628?vs=160319=161241#toc
Repository:
rC Clang
hokein added inline comments.
Comment at: clangd/XRefs.cpp:665
+std::vector references(ParsedAST , Position Pos,
+ bool IncludeDeclaration,
+ const SymbolIndex *Index) {
ilya-biryukov wrote:
> Are
hokein added a comment.
In https://reviews.llvm.org/D50896#1204310, @ilya-biryukov wrote:
> This change LG, but I would not commit it before we have an actual
> implementation.
> As soon as we have the `references` function in `ClangdUnit.cpp`
> implemented, the merge of this change should be
hokein added inline comments.
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+ auto Filename = FileEntry->getName();
+ llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
lebedev.ri
hokein added inline comments.
Comment at: clang-tidy/abseil/AbseilMatcher.h:16
+
+/// Matches AST nodes that were expanded within abseil-header-files.
+AST_POLYMORPHIC_MATCHER(isExpansionNotInAbseilHeader,
nit: We need proper documentation for this matcher,
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
The patch looks good. I'll commit for you once @JonasToth has no further
comments.
https://reviews.llvm.org/D50862
___
cfe-commits mailing list
hokein added a subscriber: deannagarcia.
hokein added a comment.
Your patch seems have some comments unaddressed but you marked done.
About the abseil matcher stuff, you and @deannagarcia have an overlap, maybe
just wait for https://reviews.llvm.org/D50580 to submit, and reuse the matcher
in
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added a subscriber: xazax.hun.
It introduces some false positives which are non-trivial to fix.
Ignore running the check in C++17.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51041
Files:
hokein updated this revision to Diff 161891.
hokein added a comment.
Use the std::equal to replace the memcmp.
Repository:
rC Clang
https://reviews.llvm.org/D50967
Files:
lib/Frontend/PrecompiledPreamble.cpp
Index: lib/Frontend/PrecompiledPreamble.cpp
hokein added a comment.
Please make sure the code follows the LLVM code style, e.g. the variable name
format "CamelName".
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hokein updated this revision to Diff 161893.
hokein added a comment.
Remove redundant CPlusPlus2a.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51041
Files:
clang-tidy/misc/UnusedUsingDeclsCheck.cpp
test/clang-tidy/misc-unused-using-decls-cxx17.cpp
Index:
hokein marked an inline comment as done.
hokein added inline comments.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:35
+ // it is not ture in C++17's template argument deduction.
+ if (!getLangOpts().CPlusPlus || getLangOpts().CPlusPlus17 ||
+
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks! Looks good.
Comment at: clangd/ClangdServer.cpp:73
+
+ void onPreambleAST(PathRef Path, ASTContext ,
+ std::shared_ptr PP) override {
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:
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
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
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 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
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
hokein added inline comments.
Comment at: clangd/index/Index.h:377
+ llvm::ArrayRef find(const SymbolID ) 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
hokein added a comment.
In https://reviews.llvm.org/D50389#1204514, @Eugene.Zelenko wrote:
> Somehow documentation file was not committed.
Oops, I forgot to `git add` to the doc file. `arc patch` somehow failed to
apply this patch, I applied it manually. Added in
hokein added a comment.
This patch is incomplete, and it is a **prototype** based on our discussion
last week. You can patch it locally, and play around with VSCode.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
___
hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, ilya-biryukov, xazax.hun.
[clangd] Simplify the code using UniqueStringSaver, NFC.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50961
Files:
hokein created this revision.
hokein added reviewers: ilya-biryukov, ioeric.
Herald added subscribers: kadircet, arphaman, mgrang, jkorous, MaskRay,
javed.absar, mgorny.
Depends on the AST callback interfaces.
- https://reviews.llvm.org/D50847
- https://reviews.llvm.org/D50889
Repository:
hokein updated this revision to Diff 161446.
hokein added a comment.
Update
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50961
Files:
clangd/index/Index.cpp
clangd/index/Index.h
Index: clangd/index/Index.h
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE340156: [clangd] Add missing lock in the lookup. (authored
by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50960?vs=161442=161447#toc
Repository:
rCTE Clang Tools
hokein added a comment.
The interfaces look mostly good to me.
Comment at: clangd/ClangdServer.cpp:73
+
+ void onPreambleAST(PathRef Path, ASTContext ,
+ std::shared_ptr PP) override {
I think `Ctx` should be a `pointer` which gives us a
hokein added inline comments.
Herald added a subscriber: kadircet.
Comment at: clangd/index/FileIndex.h:70
+ void
+ update(PathRef Path, ASTContext *AST, std::shared_ptr PP,
+ llvm::Optional> TopLevelDecls = llvm::None);
Any strong reason to unify the
hokein updated this revision to Diff 161441.
hokein added a comment.
More cleanups.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/Protocol.cpp
hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50960
Files:
clangd/index/MemIndex.cpp
Index: clangd/index/MemIndex.cpp
hokein updated this revision to Diff 161445.
hokein added a comment.
Correct the patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50961
Files:
clangd/index/Index.cpp
clangd/index/Index.h
docs/clang-tidy/checks/abseil-duration-division.rst
Index:
hokein closed this revision.
hokein added a comment.
committed in https://reviews.llvm.org/rL340161.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Passing nullptr to memcmp is UB.
Repository:
rC Clang
https://reviews.llvm.org/D50967
Files:
lib/Frontend/PrecompiledPreamble.cpp
Index: lib/Frontend/PrecompiledPreamble.cpp
hokein added inline comments.
Comment at: lib/Support/StringSaver.cpp:14
StringRef StringSaver::save(StringRef S) {
+ if (S.empty())
ioeric wrote:
> Is it possible to reuse `StringRef::copy(Allocator &)` here?
Unfortunately, not, `StringRef::copy` will
501 - 600 of 4279 matches
Mail list logo
