ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:144
+ void run(const ast_matchers::MatchFinder::MatchResult &Result) override {
+if (const auto *CMD =
+Result.Nodes.getNodeAs("class_method")) {
It'd be more readable if you pull
ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:273
}
+bool IsNamespaceStart = false;
while (DeclIt != DeclNamespaces.end()) {
It's been a while since the last time I reviewed this, and I need to struggle
to understand what Curre
This revision was automatically updated to reflect the committed changes.
Closed by commit rL286486: Handle adding new nested namespace in old namespace.
(authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D26456?vs=77511&id=77513#toc
Repository:
rL LLVM
https://reviews.
Author: ioeric
Date: Thu Nov 10 12:29:01 2016
New Revision: 286486
URL: http://llvm.org/viewvc/llvm-project?rev=286486&view=rev
Log:
Handle adding new nested namespace in old namespace.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D26456
Modified:
ioeric updated this revision to Diff 77511.
ioeric added a comment.
- Merged origin/master
https://reviews.llvm.org/D26456
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
ioeric updated this revision to Diff 77509.
ioeric added a comment.
- Address comments.
https://reviews.llvm.org/D26456
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
ioeric added inline comments.
Comment at: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp:396
+ // Calculate the name of the `NsDecl` after it is moved to new namespace.
+ std::string OldNs = NsDecl->getQualifiedNameAsString();
+ llvm::StringRef Postfix = OldNs;
-
Author: ioeric
Date: Thu Nov 10 12:15:34 2016
New Revision: 286485
URL: http://llvm.org/viewvc/llvm-project?rev=286485&view=rev
Log:
[change-namespace] dyn_cast -> cast.
Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
Modified: clang-tools-extra/trunk/change-namespace/
ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:417
+ if (const auto *FTD = CMD->getDescribedFunctionTemplate())
+UnremovedDeclsInOldHeader.erase(FTD);
+ else
hokein wrote:
> ioeric wrote:
> > `erase(FTD ? FTD : CMD)`
> We can
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg with a few nits.
Comment at: clang-move/ClangMove.cpp:417
+ if (const auto *FTD = CMD->getDescribedFunctionTemplate())
+UnremovedDeclsInOldHeader.erase(FTD);
ioeric updated this revision to Diff 77380.
ioeric added a comment.
- Added a test case with type references.
https://reviews.llvm.org/D26456
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTes
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
https://reviews.llvm.org/D26456
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
=
ioeric added a comment.
Nice!
First round of comments.
Comment at: clang-move/ClangMove.cpp:414
RemovedDecls.push_back(MovedDecls.back());
+ if (const auto *FTD = CMD->getDescribedFunctionTemplate()) {
+UnremovedDeclsInOldHeader.erase(FTD);
This revision was automatically updated to reflect the committed changes.
Closed by commit rL286307: [change-namespace] shorten namespace qualifier based
on UsingDecl and… (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D25771?vs=77270&id=77272#toc
Repository:
rL LLVM
Author: ioeric
Date: Tue Nov 8 16:44:17 2016
New Revision: 286307
URL: http://llvm.org/viewvc/llvm-project?rev=286307&view=rev
Log:
[change-namespace] shorten namespace qualifier based on UsingDecl and
UsingDirectiveDecl.
Summary:
when replacing symbol references in moved namespaces, trying to
ioeric updated this revision to Diff 77270.
ioeric marked an inline comment as done.
ioeric added a comment.
- Addressed comment.
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.
ioeric marked an inline comment as done.
ioeric added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:275
+ (DiffOldNsSplitted.empty() ? "-" : DiffOldNsSplitted.front()))
+ .str();
+ auto IsInMovedNs =
hokein wrote:
> Using an in
ioeric updated this revision to Diff 77228.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Addressed comments.
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg with a few nits.
Comment at: clang-move/ClangMove.cpp:539
+ const FileEntry *FE = SM.getFileManager().getFile(
+ MakeAbsolutePath(OriginalRunningDirectory, OldFile))
ioeric updated this revision to Diff 77115.
ioeric added a comment.
- Get rid of redundant PrefixRef
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-
ioeric added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:277
+ auto IsVisibleInOldNs =
+ anyOf(IsInMovedNs, unless(hasAncestor(namespaceDecl(hasName(Prefix);
+ // Match using declarations.
hokein wrote:
> Ignoring using-decls in `
ioeric updated this revision to Diff 77114.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Ignore using decls in old ns but not the inner-most old ns.
- Add a missing test case.
- Addressed comments
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cp
ioeric added inline comments.
Comment at: unittests/clang-move/ClangMoveTests.cpp:278
+ const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }";
+ Spec.Names = {std::string("A")};
+ Spec.OldHeader = "foo.h";
hokein wrote:
> ioeric wrote:
> > Maybe j
ioeric added a comment.
First round of comments.
The patch summary says "one moved declaration." Why it is just one moved
declaration? Doesn't this also support moving all classes in one file?
Maybe also mention the new behavior in the class comment.
Comment at: clang-move/
/asan/asan_new_delete.cc:82
#1 0xc83f64 in
clang::tooling::DeduplicateByFileTest_NonExistingFilePath_Test::TestBody()
/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/unittests/Tooling/Re
On Sun, Nov 6, 2016 at 10:08 PM, Eric Liu via cfe-commits <
cfe
Author: ioeric
Date: Mon Nov 7 12:40:41 2016
New Revision: 286132
URL: http://llvm.org/viewvc/llvm-project?rev=286132&view=rev
Log:
Fix memory leak caused by r286096.
Modified:
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
URL:
ht
cc:82
> #1 0xc83f64 in
> clang::tooling::DeduplicateByFileTest_NonExistingFilePath_Test::TestBody()
> /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/unittests/Tooling/Re
>
>
> On Sun, Nov 6, 2016 at 10:08 PM, Eric Liu via cfe-commits <
> c
ioeric marked an inline comment as done.
ioeric added a comment.
ping.
https://reviews.llvm.org/D25771
___
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 rL286096: Deduplicate replacements by FileEntry instead of
file names. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D26288?vs=77008&id=77009#toc
Repository:
rL LLVM
https://
Author: ioeric
Date: Mon Nov 7 00:08:23 2016
New Revision: 286096
URL: http://llvm.org/viewvc/llvm-project?rev=286096&view=rev
Log:
Deduplicate replacements by FileEntry instead of file names.
Summary:
The current version does not deduplicate equivalent file paths correctly.
For example, a relat
ioeric updated this revision to Diff 77008.
ioeric added a comment.
- Merge with origin/master
https://reviews.llvm.org/D26288
Files:
include/clang/Tooling/Core/Replacement.h
lib/Tooling/Core/Replacement.cpp
lib/Tooling/Refactoring.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unitt
ioeric added a comment.
Since Manuel's comment has been addressed, I'd like to land this if there is no
objection.
https://reviews.llvm.org/D26288
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
ioeric updated this revision to Diff 76975.
ioeric added a comment.
- addressed comments.
https://reviews.llvm.org/D26288
Files:
include/clang/Tooling/Core/Replacement.h
lib/Tooling/Core/Replacement.cpp
lib/Tooling/Refactoring.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/
ioeric added a comment.
In https://reviews.llvm.org/D26288#587513, @klimek wrote:
> In https://reviews.llvm.org/D26288#586932, @ioeric wrote:
>
> > - Addressed comments: handle non-existing files.
>
>
> We're not really handling them now though? We're just printing an error?
>
> My point is that
ioeric added a comment.
In https://reviews.llvm.org/D26288#586902, @klimek wrote:
> If the files do not exist, how does this work? Won't that potentially give
> different results if the files exist locally vs if they don't?
This would be problematic. Thanks for the catch!
https://reviews.llv
ioeric updated this revision to Diff 76894.
ioeric added a comment.
- Addressed comments: handle non-existing files.
https://reviews.llvm.org/D26288
Files:
include/clang/Tooling/Core/Replacement.h
lib/Tooling/Core/Replacement.cpp
lib/Tooling/Refactoring.cpp
unittests/Tooling/Refactoring
ioeric added inline comments.
Comment at: lib/Tooling/Core/Replacement.cpp:578
+ continue;
+ProcessedFileEntries.insert(FE);
+Result[Entry.first] = std::move(Entry.second);
alexshap wrote:
> (saves 2 lines of code) (although not particularly importan
ioeric updated this revision to Diff 76888.
ioeric added a comment.
- Kill a few lines.
https://reviews.llvm.org/D26288
Files:
include/clang/Tooling/Core/Replacement.h
lib/Tooling/Core/Replacement.cpp
lib/Tooling/Refactoring.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/To
ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
The current version does not deduplicate equivalent file paths correctly.
For example, a relative path and an absolute path are considered inequivalent.
Compar
Author: ioeric
Date: Mon Oct 31 03:28:29 2016
New Revision: 285549
URL: http://llvm.org/viewvc/llvm-project?rev=285549&view=rev
Log:
[change-namespace] fix namespace specifiers of template arguments.
Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/
ioeric accepted this revision.
ioeric added inline comments.
Comment at: test/clang-tidy/modernize-use-default-copy.cpp:85
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+ // CHECK-FIXES: /* don't delete */ = default;
int Field;
malcolm.parso
ioeric marked 3 inline comments as done.
ioeric added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:566
+ break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start))
{
+ for (const auto *UsingShadow : Using->shadows()) {
-
ioeric updated this revision to Diff 75281.
ioeric added a comment.
- Add a missing test case.
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namesp
ioeric added a comment.
Still missing one case... working on a fix.
https://reviews.llvm.org/D25771
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric updated this revision to Diff 75280.
ioeric added a comment.
- Ignore using decls in old ns but not the inner-most old ns.
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.
ioeric added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:566
+ break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start))
{
+ for (const auto *UsingShadow : Using->shadows()) {
ioeric wrote:
> hokein wr
ioeric added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:566
+ break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start))
{
+ for (const auto *UsingShadow : Using->shadows()) {
hokein wrote:
> Yeah, it
ioeric abandoned this revision.
ioeric added a comment.
Thanks all!
https://reviews.llvm.org/D25597
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
when replacing symbol references in moved namespaces, trying to make the replace
name as short as possible by considering UsingDecl (i.e. UsingShadow) and
UsingDirectiveDecl (i.e. using namespace
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg
Comment at: clang-move/ClangMove.cpp:372
+ // Match static functions/variabale definitions which are defined in named
+ // spaces.
+ auto IsOldCCStaticDefinition =
ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:356
Finder->addMatcher(
- namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())),
-inAnonymousNamespace)
- .bind("decls_in_anonymous_ns"),
+ usingDecl(InOldCC,
Author: ioeric
Date: Wed Oct 19 03:19:46 2016
New Revision: 284573
URL: http://llvm.org/viewvc/llvm-project?rev=284573&view=rev
Log:
[clang-format] Add comment manipulation header
Summary:
Introduces a separate target for comment manipulation.
Currently, comment manipulation is in BreakableCommen
clang-apply-replacements has some legacy code and needs some refactoring in
order to take advantage of the cleanup feature (because of the new
tooling::Replacements implementation). I had a plan to refactor it but
thought it was a low priority since most refactoring tools are using
replacements app
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Awesome! Lg with one nit.
Comment at: clang-move/ClangMove.cpp:197
clang::tooling::Replacement
getReplacementInChangedCode(const clang::tooling::Replacements &Replacements,
ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:472
std::string FilePath = RemoveReplacement.getFilePath().str();
addOrMergeReplacement(RemoveReplacement, &FileToReplacements[FilePath]);
For deletions, you can simply use `add`, whic
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg
Comment at: clang-move/ClangMove.cpp:264
+HeaderGuard += "#define " + GuardName + "\n";
+clang::tooling::Replacement HeaderGuardInclude(FileName, 0, 0,
+
Author: ioeric
Date: Fri Oct 14 06:48:10 2016
New Revision: 284228
URL: http://llvm.org/viewvc/llvm-project?rev=284228&view=rev
Log:
Removed duplicate header include
Reviewers: ioeric
Subscribers: klimek
Patch by Krasimir Georgiev!
Differential Revision: https://reviews.llvm.org/D25599
Modifi
Author: ioeric
Date: Fri Oct 14 05:10:26 2016
New Revision: 284222
URL: http://llvm.org/viewvc/llvm-project?rev=284222&view=rev
Log:
Try to fix windows bot file path style failure caused by r284219.
Modified:
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
Modified: cfe/trunk/unittests/Tooli
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D25598
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
Author: ioeric
Date: Fri Oct 14 04:32:06 2016
New Revision: 284219
URL: http://llvm.org/viewvc/llvm-project?rev=284219&view=rev
Log:
Deduplicate sets of replacements by file names.
Summary:
If there are multiple pairs with the same file
path after removing dots, we only keep one pair (with path
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284219: Deduplicate sets of replacements by file names.
(authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D25565?vs=74634&id=74636#toc
Repository:
rL LLVM
https://reviews.llvm.
ioeric updated this revision to Diff 74634.
ioeric added a comment.
- Merge branch 'master' of http://llvm.org/git/clang into arcpatch-D25565
- Forgot to update names in tests...
https://reviews.llvm.org/D25565
Files:
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/Refactorin
ioeric updated this revision to Diff 74632.
ioeric added a comment.
- Change name to groupReplacementsByFile
https://reviews.llvm.org/D25565
Files:
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/Refactoring.h
lib/Tooling/Core/Replacement.cpp
lib/Tooling/Refactoring.cpp
ioeric updated this revision to Diff 74631.
ioeric added a comment.
- Separate assertions to get more information.
https://reviews.llvm.org/D25597
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
==
ioeric created this revision.
ioeric added a reviewer: bkramer.
ioeric added a subscriber: cfe-commits.
Since `remove_dots` does not delete leading "../" anymore, assertion test need
to be updated.
https://reviews.llvm.org/D25597
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/Virtu
Author: ioeric
Date: Thu Oct 13 14:49:19 2016
New Revision: 284155
URL: http://llvm.org/viewvc/llvm-project?rev=284155&view=rev
Log:
[clang-move] error out when fail to create new files.
Modified:
clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp
Modified: clang-tools-extra/trunk/cla
Author: ioeric
Date: Thu Oct 13 14:04:19 2016
New Revision: 284148
URL: http://llvm.org/viewvc/llvm-project?rev=284148&view=rev
Log:
Print stack trace for clang-move tool.
Modified:
clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp
Modified: clang-tools-extra/trunk/clang-move/tool/Cl
Author: ioeric
Date: Thu Oct 13 13:56:14 2016
New Revision: 284147
URL: http://llvm.org/viewvc/llvm-project?rev=284147&view=rev
Log:
Print stack trace for clang-change-namespace tool.
Modified:
clang-tools-extra/trunk/change-namespace/tool/ClangChangeNamespace.cpp
Modified: clang-tools-extra
ioeric added inline comments.
Comment at: include/clang/Tooling/Core/Replacement.h:233
- Replacements mergeReplacements(const ReplacementsImpl &Second) const;
-
This is dead code.
https://reviews.llvm.org/D25565
___
ioeric updated this revision to Diff 74531.
ioeric added a comment.
- Updated comments for RefactoringTool.
https://reviews.llvm.org/D25565
Files:
include/clang/Tooling/Core/Replacement.h
include/clang/Tooling/Refactoring.h
lib/Tooling/Core/Replacement.cpp
lib/Tooling/Refactoring.cpp
ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added subscribers: cfe-commits, bkramer, hokein.
Herald added a subscriber: klimek.
If there are multiple pairs with the same file
path after removing dots, we only keep one pair (with path after dots being
removed) and discar
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg with one nit.
Comment at: clang-move/ClangMove.cpp:410
std::string AbsolutePath = MakeAbsolutePath(SM, FileName);
if (MakeAbsolutePath(OriginalRunningDirectory, Spec
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284011: [change-namespace] don't miss comments in the
beginning of a namespace block. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D25397?vs=74038&id=74368#toc
Repository:
Author: ioeric
Date: Wed Oct 12 07:34:18 2016
New Revision: 284011
URL: http://llvm.org/viewvc/llvm-project?rev=284011&view=rev
Log:
[change-namespace] don't miss comments in the beginning of a namespace block.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.ll
ioeric added inline comments.
Comment at: change-namespace/ChangeNamespace.cpp:387
+ Token Tok;
+ while (!Lex->LexFromRawLexer(Tok) && Tok.isNot(tok::TokenKind::l_brace)) {
+ }
hokein wrote:
> Maybe we can use `findLocationAfterToken` here?
This doesn't seem
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D24572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
https://reviews.llvm.org/D25397
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
==
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D25369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:27
+AST_MATCHER_P(CXXMethodDecl, ofOutermostEnclosingClass,
+ ast_matchers::internal::Matcher, InnerMatcher) {
I'm not sure if we really need to limit this to the `outer-most` class
ioeric added inline comments.
Comment at: clang-move/ClangMove.cpp:316
auto InMovedClass =
- hasDeclContext(cxxRecordDecl(hasName(FullyQualifiedName)));
+ hasDeclContext(cxxRecordDecl(*InMovedClassNames));
ioeric wrote:
> hokein wrote:
> > ioeric w
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg. Thanks!
https://reviews.llvm.org/D25309
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
ioeric added inline comments.
> hokein wrote in ClangMove.cpp:316
> "InMovedClassNames" should not be false in the matcher. Added an assert.
I think you should instead exit early if `ClassNames.empty()` or assert not
empty.
> ClangMove.cpp:299
> + for (StringRef ClassName : ClassNames) {
> +
ioeric added inline comments.
> ClangMove.cpp:295
> void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
> - std::string FullyQualifiedName = "::" + Spec.Name;
> + auto ParseNames = [](llvm::StringRef Names) {
> +SmallVector ClassNames;
Why create this lambda if it's
ioeric added a comment.
Test phab...sorry spamming...
https://reviews.llvm.org/D24380
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D25227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Lg
https://reviews.llvm.org/D25282
___
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 rL28: [change-namespace] Fixed a bug in
getShortestQualifiedNameInNamespace. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D25065?vs=73459&id=73653#toc
Repository:
rL LLVM
Author: ioeric
Date: Wed Oct 5 10:52:39 2016
New Revision: 28
URL: http://llvm.org/viewvc/llvm-project?rev=28&view=rev
Log:
[change-namespace] Fixed a bug in getShortestQualifiedNameInNamespace.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/
This revision was automatically updated to reflect the committed changes.
Closed by commit rL283332: Make DeletedLines local variables in
checkEmptyNamespace. (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D25162?vs=73641&id=73652#toc
Repository:
rL LLVM
https://rev
Author: ioeric
Date: Wed Oct 5 10:49:01 2016
New Revision: 283332
URL: http://llvm.org/viewvc/llvm-project?rev=283332&view=rev
Log:
Make DeletedLines local variables in checkEmptyNamespace.
Summary: Patch by Sam McCall!
Reviewers: djasper
Subscribers: klimek, cfe-commits
Differential Revision
This revision was automatically updated to reflect the committed changes.
Closed by commit rL283330: [clang-format] append newline after code when
inserting new headers at the end… (authored by ioeric).
Changed prior to commit:
https://reviews.llvm.org/D21026?vs=73648&id=73651#toc
Repository:
Author: ioeric
Date: Wed Oct 5 10:42:19 2016
New Revision: 283330
URL: http://llvm.org/viewvc/llvm-project?rev=283330&view=rev
Log:
[clang-format] append newline after code when inserting new headers at the end
of the code which does not end with newline.
Summary:
append newline after code when
ioeric updated this revision to Diff 73648.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Merged with origin/master.
- Update newline insertion according to the new Replacements implementation.
https://reviews.llvm.org/D21026
Files:
lib/Format/Format.cpp
unittests/Forma
ioeric added a comment.
Sorry for the delay
I've updated the patch to work with the new tooling::Replacements.
https://reviews.llvm.org/D21026
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
ioeric updated this revision to Diff 73641.
ioeric added a comment.
- Added a test case.
https://reviews.llvm.org/D25162
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
==
ioeric added inline comments.
> hokein wrote in ChangeNamespace.cpp:187
> I think the statement doesn't compile here, since `consume_front` return a
> `bool`. It should be `if (DeclName.consume_front((NsName + "::")))`?
>
> Looks like we can also put this judge into the above `while` statement?
ioeric updated this revision to Diff 73459.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Address review comments.
https://reviews.llvm.org/D25065
Files:
change-namespace/ChangeNamespace.cpp
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-n
ioeric added a comment.
This is great!
> ClangMove.cpp:70-74
> + if (SM->getLocForEndOfFile(LocInfo.first)
> + == EndLoc)
> +return EndLoc;
> + // Include the trailing newline character "\n".
> + return EndLoc.getLocWithOffset(1);
Maybe:
return (SM->getLocForEndOfFile(LocInfo.fi
ioeric updated this revision to Diff 73446.
ioeric added a comment.
Herald added a subscriber: modocache.
- Add SymbolRenameSpec; replace addIncludesToFiles and renameSymbolsInFiles
with renameSymbolsInAffectedFiles.
https://reviews.llvm.org/D24380
Files:
CMakeLists.txt
migrate-tool/Affect
This is not due to the upgrade. There was long delay before Phabricator
receives a commit, but the delay seems to be gone now. Let me know if you
are still experiencing the delay.
- Eric
On Mon, Oct 3, 2016 at 4:25 PM Michał Górny wrote:
> On Mon, 3 Oct 2016 13:47:08 +
> Sjoerd Meijer via c
401 - 500 of 970 matches
Mail list logo
