ioeric added a comment.
In https://reviews.llvm.org/D40548#937626, @malaperle wrote:
> Hi Eric! As you might know I'm working on persisted indexing. I was wondering
> which cases needed the index for code completion? Could you give a small
> example? I thought the AST alone would be sufficient
ioeric created this revision.
... in qualified code completion and decl lookup.
https://reviews.llvm.org/D40562
Files:
lib/Sema/SemaCodeComplete.cpp
lib/Sema/SemaLookup.cpp
test/CodeCompletion/ignore-global-decls.cpp
Index: test/CodeCompletion/ignore-global-decls.cpp
===
ioeric created this revision.
https://reviews.llvm.org/D40563
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sem
ioeric added a comment.
First round of comments. Mostly around indexing actions and file records; I
haven't started reviewing the data writing and storing code. I think it might
make sense to split the index writing and storing logics into a separate patch,
which should be possible if `writeUni
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/Protocol.cpp:56
+assert(*this && "Must check this is an object before calling parse()");
+if (const json::Expr *E = O->get(Prop)) {
+ ret
ioeric updated this revision to Diff 124739.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Address review comments.
https://reviews.llvm.org/D40563
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
=
ioeric added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4609
+ if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Why do we alter this code path?
> Maybe we shou
ioeric updated this revision to Diff 124740.
ioeric added a comment.
- Clarify comment for includeGlobals()
https://reviews.llvm.org/D40562
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
lib/Sema/SemaLookup.cpp
test/CodeCompletion/ignore-global-decls.cpp
ioeric updated this revision to Diff 124745.
ioeric added a comment.
Herald added a subscriber: klimek.
Merged with origin/master
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/ASTIndex.cpp
clangd/ASTIndex.h
clangd/CMakeLists.txt
clangd/ClangdIndex.c
ioeric added a comment.
In https://reviews.llvm.org/D40563#939555, @arphaman wrote:
> Could you please add a test?
Any tip on how this should be tested? I couldn't find any existing unit test
for either SemaCodeComplete or code completion context (under
`clang/unittests`). It might be possibl
ioeric added a comment.
In https://reviews.llvm.org/D40563#939964, @arphaman wrote:
> If nothing uses `getCXXScopeSpecifier` right now we can't really test it with
> a clang or c-index-test regression test. A completion unit test could work
> here. I don't think we actually have existing comple
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
ioeric added a comment.
In https://reviews.llvm.org/D40562#941570, @arphaman wrote:
> In https://reviews.llvm.org/D40562#940201, @ilya-biryukov wrote:
>
> > In https://reviews.llvm.org/D40562#939950, @arphaman wrote:
> >
> > > This change breaks cached completions for declarations in namespaces i
ioeric added inline comments.
Comment at: clangd/ClangdUnit.cpp:377
+ : Result(&Result), SymbolScore(score(Result)), FilterScore(FilterScore),
+Score(FilterScore * SymbolScore) {}
It might worth mentioning how well `FilterScore * SymbolScore` perfo
ioeric created this revision.
This enables us to use information in Preprocessor when handling symbol
occurrences.
Repository:
rC Clang
https://reviews.llvm.org/D40884
Files:
include/clang/Index/IndexDataConsumer.h
include/clang/Index/IndexingAction.h
lib/Index/IndexingAction.cpp
too
ioeric added inline comments.
Comment at: clangd/Context.h:63
+/// used as parents for some other Contexts.
+class Context {
+public:
sammccall wrote:
> I think we should strongly consider calling the class Ctx over Context. It's
> going to appear in many functi
ioeric added a comment.
In https://reviews.llvm.org/D40884#946506, @malaperle wrote:
> You can get the preprocessor from the ASTContext, no?
I don't think `ASTContext` contains preprocessor information.
Repository:
rC Clang
https://reviews.llvm.org/D40884
___
ioeric updated this revision to Diff 125777.
ioeric added a comment.
- Add a new code-completion option IncludeNamespaceLevelDecls. For now, I only
restrict this option work for qualified id completion to reduce the impact.
Repository:
rC Clang
https://reviews.llvm.org/D40562
Files:
inclu
ioeric added a comment.
In https://reviews.llvm.org/D40562#942521, @arphaman wrote:
> In https://reviews.llvm.org/D40562#941753, @ilya-biryukov wrote:
>
> > In https://reviews.llvm.org/D40562#941570, @arphaman wrote:
> >
> > > I'm not actually 100% sure, but I would imagine that this one of the
ioeric added a comment.
Hi Marc, the patch is not ready for review yet. I am still cleaning up the
prototype and will let you know when it's ready for review.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
___
cfe-commits mai
ioeric added inline comments.
Comment at: clangd/Symbol.h:23
+ // The path of the source file where a symbol occurs.
+ std::string FilePath;
+ // The offset to the first character of the symbol from the beginning of the
Is this relative or absolute?
ioeric updated this revision to Diff 125913.
ioeric marked an inline comment as done.
ioeric added a comment.
- Removed a redundant #include
Repository:
rC Clang
https://reviews.llvm.org/D40884
Files:
include/clang/Index/IndexDataConsumer.h
lib/Index/IndexingAction.cpp
tools/libclang/C
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320030: [Index] Add setPreprocessor member to
IndexDataConsumer. (authored by ioeric).
Repository:
rL LLVM
https://reviews.llvm.org/D40884
Files:
cfe/trunk/include/clang/Index/IndexDataConsumer.h
This revision was automatically updated to reflect the committed changes.
Closed by commit rC320030: [Index] Add setPreprocessor member to
IndexDataConsumer. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D40884?vs=125913&id=125920#toc
Repository:
rC Clang
https://r
ioeric updated this revision to Diff 125928.
ioeric added a comment.
- More cleanups and merged with https://reviews.llvm.org/D40897
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/CMakeLists.txt
clangd/ClangdIndex.cpp
clangd/ClangdIndex.h
clangd/Clan
ioeric updated this revision to Diff 125960.
ioeric added a comment.
- Use IncludeNamespaceLevelDecls option; fix some broken tests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/CMakeLists.txt
clangd/ClangdIndex.cpp
clangd/ClangdIndex.h
clangd/Clan
ioeric updated this revision to Diff 125962.
ioeric added a comment.
Diff on https://reviews.llvm.org/D40897 instead origin/master!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/CMakeLists.txt
clangd/ClangdIndex.cpp
clangd/ClangdIndex.h
clangd/Clang
ioeric added a comment.
Ping.
(fyi, you could find use of the new option in https://reviews.llvm.org/D40548)
Repository:
rC Clang
https://reviews.llvm.org/D40562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-b
ioeric added a comment.
Fyi, you could find use of the new API in https://reviews.llvm.org/D40548
I'd like to land this patch if there is no objection.
https://reviews.llvm.org/D40563
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
ioeric created this revision.
Herald added subscribers: cfe-commits, klimek.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41001
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespac
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320139: [change-namespace] Fix crash when injected
base-class name is used in friend… (authored by ioeric).
Repository:
rL LLVM
https://reviews.llvm.org/D41001
Files:
clang-tools-extra/trunk/change-
ioeric added a comment.
In https://reviews.llvm.org/D40548#949182, @malaperle wrote:
> As a follow-up, here's the interface for querying the index that I am using
> right now. It's meant to be able to retrieve from any kind of "backend", i.e.
> in-memory, ClangdIndexDataStore, libIndexStore, et
ioeric accepted this revision.
ioeric added a comment.
lg
Comment at: unittests/clangd/Matchers.h:54
+ bool MatchAndExplain(const std::vector &V,
+ ::testing::MatchResultListener *L) const {
+std::vector Matches(Matchers.size());
`ove
ioeric updated this revision to Diff 126379.
ioeric marked 8 inline comments as done.
ioeric added a comment.
- Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/CMakeLists.txt
clangd/ClangdIndex.cpp
clangd/ClangdIndex.h
clangd/
ioeric added inline comments.
Comment at: clangd/ClangdIndex.h:1
+//===--- ClangdIndex.h - Symbol indexes for clangd.---*-
C++-*-===//
+//
sammccall wrote:
> nit: `Clangd` prefix doesn't do much.
> I'd suggest `Index.h` for the interface (including S
ioeric added a comment.
Thanks a lot for the changes! Some more comments inlined.
Please mark addressed comments as done so that reviewers could know what to
look :) Thanks!
Comment at: include/clang/Frontend/CompilerInstance.h:187
+ typedef std::function(
+ const Front
ioeric added a comment.
Ping?
Repository:
rC Clang
https://reviews.llvm.org/D40562
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320471: [SemaCodeComplete] Allow passing out scope
specifiers in qualified-id… (authored by ioeric, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D40563
Files:
cfe/trunk/include/clang
ioeric added inline comments.
Comment at: clangd/index/Index.h:52
+private:
+ friend class llvm::DenseMapInfo;
+
warning: class 'DenseMapInfo' was previously declared as a struct
[-Wmismatched-tags]
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.o
ioeric updated this revision to Diff 126686.
ioeric added a comment.
Merged with origin/master and moved index-related files to index/ subdirectory.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdL
ioeric updated this revision to Diff 126694.
ioeric added a comment.
- Merged with origin/master
Repository:
rC Clang
https://reviews.llvm.org/D40562
Files:
include/clang/Driver/CC1Options.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/CodeCompleteOptions.h
lib/Front
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 rL320563: [Sema] Ignore decls in namespaces when global decls
are not wanted. (authored by ioeric, committed by ).
Changed
ioeric updated this revision to Diff 126775.
ioeric added a comment.
- Remove everything except for SymbolIndex interface and MemIndex
- Added unit tests for MemIndex
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40548
Files:
clangd/CMakeLists.txt
clangd/index/Index.cpp
ioeric updated this revision to Diff 108881.
ioeric added a comment.
- Merged with origin/master
https://reviews.llvm.org/D30777
Files:
include/clang/Tooling/Refactoring/AtomicChange.h
lib/Tooling/Refactoring/AtomicChange.cpp
lib/Tooling/Refactoring/CMakeLists.txt
unittests/Tooling/Refa
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309548: Added `applyAtomicChanges` function. (authored by
ioeric).
Repository:
rL LLVM
https://reviews.llvm.org/D30777
Files:
cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h
cfe/trunk/l
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Yep. Lgtm!
Repository:
rL LLVM
https://reviews.llvm.org/D35012
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Thanks!!!
https://reviews.llvm.org/D36149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
ioeric added a comment.
This is great work and definitely a lot to digest! ;) Some high-level comments
for the first round.
In general, I really appreciate the high-level interfaces; I am just a bit
concerned about the implementation which is a bit hard to follow at this point,
especially with
ioeric added a comment.
Thanks for the changes! The code is much clearer.
I am wondering if the current design could be extended to support tools (or
rules) that use AST matchers? Or is the selection expected to be powerful
enough to replace AST matchers?
We have a few tools in `clang-tools-ex
ioeric added inline comments.
Comment at:
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template
+detail::SourceSelectionRequirement<
+typename selection::detail::EvaluateSelectionChecker<
arphaman wrote:
> ioeric wrote:
> > Coul
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at:
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:75
+/// \brief Deduplicate, check for conflicts, and convert all Repla
ioeric added inline comments.
Comment at: include/clang/Tooling/Execution.h:76
+
+ void appendArgumentsAdjuster(ArgumentsAdjuster Adjuster);
+
klimek wrote:
> I think the argument adjuster adjustment shouldn't be part of this interface,
> as the argument adjust
ioeric updated this revision to Diff 120370.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Addressed review comments o Removed argument adjusting interfaces from
ExecutionContext o Switched positions of Action and Adjuster parameters in the
`execute` interfaces
https://rev
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316653: [Tooling] A new framework for executing clang
frontend actions. (authored by ioeric).
Repository:
rL LLVM
https://reviews.llvm.org/D34272
Files:
cfe/trunk/include/clang/Tooling/CommonOptions
ioeric accepted this revision.
ioeric added a comment.
lg
Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:48
+
+ // static const RefactoringDescriptor &describe() = 0;
};
A comment would be nice here.
Comment at: lib/T
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
arphaman wrote:
> hokein wrote:
> > sammccall wrote:
> > > As discussed offline, it's not cle
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
ioeric wrote:
> arphaman wrote:
> > hokein wrote:
> > > sammccall wrote:
> > > > As discussed
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
arphaman wrote:
> ioeric wrote:
> > ioeric wrote:
> > > arphaman wrote:
> > > > hokein wrote:
ioeric added a comment.
Code looks good with some nits.
Comment at: lib/Tooling/Refactoring/CMakeLists.txt:7
AtomicChange.cpp
Extract.cpp
+ SourceExtraction.cpp
Shall we move function extraction sources into a sub-directory? Like what you
did for header
ioeric accepted this revision.
ioeric added a comment.
still lgtm
Comment at: clangd/Trace.h:38
+ // Starts a sessions capturing trace events and writing Trace Event JSON.
+ static std::unique_ptr createJSON(llvm::raw_ostream &OS);
+ ~Session();
`createJSON`
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:101
+ std::string NewQualifiedName) {
+ return QualifiedRenameRule(std::move(OldQualifiedName),
+ std::move(NewQualifiedName));
--
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D39441
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
ioeric created this revision.
This is a refactoring change. NFC
https://reviews.llvm.org/D39675
Files:
tools/clang-refactor/ClangRefactor.cpp
Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefact
ioeric updated this revision to Diff 121883.
ioeric added a comment.
- Fix typos.
https://reviews.llvm.org/D39675
Files:
tools/clang-refactor/ClangRefactor.cpp
Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-re
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317577: [clang-refactor] Use ClangTool more explicitly by
making refaroing actions AST… (authored by ioeric).
Repository:
rL LLVM
https://reviews.llvm.org/D39675
Files:
cfe/trunk/tools/clang-refacto
ioeric added a comment.
Implementation looks good. Just some nits.
Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.cpp:36
+ return true;
+// FIXME: Capture 'self'.
+if (!VD->isLocalVarDeclOrParm())
and `this`?
Commen
ioeric added a comment.
In https://reviews.llvm.org/D39050#900830, @arphaman wrote:
> I think this patch should be split into a number of smaller patches to help
> the review process.
>
> Things like `tools/IndexStore`, `DirectoryWatcher` and other components that
> are not directly needed righ
ioeric added a comment.
>> - I think the implementation of the index output logic (e.g.
>> `IndexUnitWriter` and bit format file) can be abstracted away (and split
>> into separate patches) so that you can unit-test the action with a
>> custom/mock unit writer; this would also make the action r
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Extract/CaptureVariables.cpp:97
+ }
+ llvm_unreachable("invalid kind!");
+}
A more informative message would be better.
Comment at: lib/Tooling/Refactoring/Extract/CaptureVaria
ioeric added a comment.
Drive-by comment: in general, have you considered reusing the existing
declarations and occurrences finding functionalities in clang-rename? AFAIK, it
deals with templates and macros pretty well.
o
https://github.com/llvm-mirror/clang/blob/master/include/clang/Tooling/R
ioeric added a comment.
In https://reviews.llvm.org/D39050#922597, @akyrtzi wrote:
> Hey Eric,
>
> In https://reviews.llvm.org/D39050#921748, @ioeric wrote:
>
> > >> - I think the implementation of the index output logic (e.g.
> > >> `IndexUnitWriter` and bit format file) can be abstracted away
ioeric added a comment.
>> If the index action is already flexible enough, would you mind splitting the
>> code for the index action out so that we can start reviewing it? Given that
>> the current patch has very few tests, I guess it wouldn't be too much worse
>> to split out the action withou
ioeric added a comment.
Code looks good. Just some nits.
Comment at: clangd/JSONExpr.h:62
+// Array and Object also have typed indexing accessors for easy traversal:
+// if (json::obj* Opts = O.array("options"))
+// if (Optional Font = Opts->string("font"))
--
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D40182
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289672: [change-namespace] don't crash when type reference
is in function type… (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D27758?vs=81388&id=81400#toc
Repository:
rL LLV
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
This fixes templated type aliases and templated type aliases in classes.
https://reviews.llvm.org/D27801
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamesp
ioeric updated this revision to Diff 81555.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Comments addressed
https://reviews.llvm.org/D27801
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespac
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289799: [change-namespace] handling templated type aliases
correctly. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D27801?vs=81555&id=81557#toc
Repository:
rL LLVM
https:/
ioeric added a comment.
Second rounds. Will start reviewing `CallGraph` code next.
Comment at: clang-move/ClangMove.cpp:492
+ isDefinition(), unless(InMovedClass), InOldCC,
+ anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous();
+ auto HelperFuncOr
ioeric added inline comments.
Comment at: clang-move/UsedHelperDeclFinder.cpp:22
+// by a single node which belongs to that class.
+const Decl *getOutmostEnclosingClassOrFunDecl(const Decl *D) {
+ const auto *DC = D->getDeclContext();
Maybe just `getOutermostDec
ioeric added a comment.
Code is almost good. I'm just still a bit confused by names.
Comment at: clang-move/ClangMove.cpp:459
//
- auto InOldCCNamedOrGlobalNamespace =
- allOf(hasParent(decl(a
ioeric added a comment.
`llvm::ErrorOr` carries `std::error_code`. If you want richer information (e.g.
error_code + error message), `llvm::Expcted` and `llvm::Error` are your
friends.
FYI, if you only need error_code + error_message in the returned error, there
is also `llvm::StringError`. An
ioeric added inline comments.
Comment at: test/clang-move/move-used-helper-decls.cpp:1
+// RUN: mkdir -p %T/used-helper-decls
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
Can you also add test cases where class members are treated as the same n
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
Also make sure one function reference is only processed once.
https://reviews.llvm.org/D27982
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/cha
ioeric updated this revision to Diff 82098.
ioeric added a comment.
- minor fix.
https://reviews.llvm.org/D27982
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNam
ioeric updated this revision to Diff 82104.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Address comments
https://reviews.llvm.org/D27982
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.cp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290176: [change-namespace] do not fix calls to overloaded
operator functions. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D27982?vs=82104&id=82105#toc
Repository:
rL LLVM
ioeric added a comment.
Why do we allow partially specialization but fully specialization now? Could
you elaborate in the cl description?
https://reviews.llvm.org/D27920
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
ioeric added a comment.
I'm not quite convinced/sure if we want to match all partial specializations
just for `std::function`. We filtered out template specialization because there
could be multiple specializations for a template in different headers and it
was not clear which one we would choo
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
https://reviews.llvm.org/D28052
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-
ioeric updated this revision to Diff 82345.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- addressed review comments.
https://reviews.llvm.org/D28052
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespa
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290421: [change-namespace] consider namespace aliases to
shorten qualified names. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D28052?vs=82345&id=82405#toc
Repository:
rL L
ioeric added inline comments.
Comment at: include/clang/Format/Format.h:862
///
/// \returns FormatStyle as specified by ``StyleName``. If no style could be
/// determined, the default is LLVM Style (see ``getLLVMStyle()``).
Is this still true?
=
ioeric added inline comments.
Comment at: lib/Format/Format.cpp:1890
}
- FormatStyle Style = getLLVMStyle();
- Style.Language = getLanguageByFileName(FileName);
+ FormatStyle::LanguageKind Language = getLanguageByFileName(FileName);
amaiorano wrote:
> ioe
ioeric added inline comments.
Comment at: lib/Format/Format.cpp:1984
+// If so, can't return this error here...
+return make_string_error("Configuration file(s) do(es) not support " +
+ getLanguageName(Language) + ": " +
amaior
ioeric added a comment.
In https://reviews.llvm.org/D28081#633103, @amaiorano wrote:
> Hello everyone, so after a few more tests, I've uncovered a bug and perhaps a
> different meaning for fallback style. First, the bug: if you set fallback
> style to "none", clang-format will perform no replac
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Awesome! Let's ship it!
https://reviews.llvm.org/D27673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
ioeric added a comment.
Some nits. Some is almost good :)
BTW, do you have clang-tools-extra in your source tree? There are also some
references in the subtree to the changed interface. It would be nice if you
could also fix them in a separate patch and commit these two patches together
(I mea
ioeric added a comment.
Oops, sorry about the typo. I mean, code is almost good! :)
https://reviews.llvm.org/D28081
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric added a comment.
The patch LGTM now. I'll accept both this and the one for clang-tool-extra when
it is ready.
Regarding builbots, we have bots that continually run builds/tests
(http://lab.llvm.org:8011/). Many buildbots test llvm and clang as well as
clang-tools-extra (e.g. with `ninja
1201 - 1300 of 1511 matches
Mail list logo
