Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-07-11 Thread Eric Liu via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL275062: Make tooling::applyAllReplacements return llvm::Expected instead of… (authored by ioeric). Changed prior to commit: http://reviews.llvm.org/D21601?vs=61823&id=63505#toc Repository: rL LLVM h

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-07-04 Thread Manuel Klimek via cfe-commits
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg http://reviews.llvm.org/D21601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-07-04 Thread Eric Liu via cfe-commits
ioeric added a comment. PING http://reviews.llvm.org/D21601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-30 Thread Eric Liu via cfe-commits
ioeric added a comment. Ping http://reviews.llvm.org/D21601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-28 Thread Eric Liu via cfe-commits
ioeric marked 3 inline comments as done. ioeric added a comment. Friendly ping http://reviews.llvm.org/D21601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 61823. ioeric added a comment. - Addressed reviewer's comments http://reviews.llvm.org/D21601 Files: include/clang/Format/Format.h include/clang/Tooling/Core/Replacement.h lib/Format/Format.cpp lib/Tooling/Core/Replacement.cpp lib/Tooling/Refactori

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Manuel Klimek via cfe-commits
klimek added a comment. In http://reviews.llvm.org/D21601#466513, @ioeric wrote: > Would EXPECTED_FALSE(!Code) be better? Not really. Can we at least use static_cast(...)? http://reviews.llvm.org/D21601 ___ cfe-commits mailing list cfe-commits@li

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Eric Liu via cfe-commits
Would EXPECTED_FALSE(!Code) be better? On Fri, Jun 24, 2016, 17:42 Manuel Klimek wrote: > klimek added inline comments. > > > Comment at: unittests/Format/CleanupTest.cpp:258 > @@ +257,3 @@ > +auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style); > +EXPE

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: unittests/Format/CleanupTest.cpp:258 @@ +257,3 @@ +auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style); +EXPECT_TRUE((bool)CleanReplaces) +<< llvm::toString(CleanReplaces.takeError()) << "\n";

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 61785. ioeric added a comment. - Addressed comments. http://reviews.llvm.org/D21601 Files: include/clang/Format/Format.h include/clang/Tooling/Core/Replacement.h lib/Format/Format.cpp lib/Tooling/Core/Replacement.cpp lib/Tooling/Refactoring.cpp t

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Eric Liu via cfe-commits
ioeric marked 2 inline comments as done. Comment at: unittests/Format/CleanupTest.cpp:258 @@ +257,3 @@ +auto CleanReplaces = cleanupAroundReplacements(Code, Replaces, Style); +EXPECT_TRUE((bool)CleanReplaces) +<< llvm::toString(CleanReplaces.takeError()) << "\n"; -

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: include/clang/Format/Format.h:780 @@ -778,2 +779,3 @@ /// \brief Returns the replacements corresponding to applying \p Replaces and -/// cleaning up the code after that. +/// cleaning up the code after that on success; otheriwse, return a

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Eric Liu via cfe-commits
ioeric marked 4 inline comments as done. ioeric added a comment. mark previous comments as done http://reviews.llvm.org/D21601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-24 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 61780. ioeric added a comment. - fixed commenting. - Make formatReplacemnts and cleanupAroundReplacements return llvm::Exppected<...>. http://reviews.llvm.org/D21601 Files: include/clang/Format/Format.h include/clang/Tooling/Core/Replacement.h lib/For

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-23 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: lib/Format/Format.cpp:1395 @@ -1394,2 +1394,3 @@ +// If any replacements in \p Replaces fails to apply, this returns \p Replaces. template ioeric wrote: > klimek wrote: > > Why would we want to return the same replacem

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-23 Thread Eric Liu via cfe-commits
ioeric added inline comments. Comment at: lib/Format/Format.cpp:1395 @@ -1394,2 +1394,3 @@ +// If any replacements in \p Replaces fails to apply, this returns \p Replaces. template klimek wrote: > Why would we want to return the same replacements? this is just

Re: [PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-23 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: include/clang/Tooling/Core/Replacement.h:170 @@ +169,3 @@ +/// This completely ignores the path stored in each replacement. If all +/// replacements are applied successfully, this returns the result code; +/// otherwise, an llvm::Error car

[PATCH] D21601: Make tooling::applyAllReplacements return llvm::Expected instead of empty string to indicate potential error.

2016-06-22 Thread Eric Liu via cfe-commits
ioeric created this revision. ioeric added reviewers: klimek, djasper. ioeric added a subscriber: cfe-commits. Herald added a subscriber: klimek. return llvm::Expected<> to carry error status and error information. This is the first step towards introducing "Error" into tooling::Replacements. htt