klimek added a comment.
(full disclosure: I'm also generally opposed to this change, but if there are
really enough users using this it's probably a lost cause)
https://reviews.llvm.org/D22505
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added a comment.
In https://reviews.llvm.org/D22505#503472, @LokiAstari wrote:
> I don't have a problem changing it so the default behaviour is:
>
> class C {
> int v1;
> private:
> int v2;
> };
>
>
> But I would like to retain the functionality that if there is no e
klimek created this revision.
klimek added reviewers: djasper, bkramer, ioeric.
klimek added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/Re
klimek updated this revision to Diff 66651.
klimek added a comment.
Remove re-implementation of overlaps check.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/RefactoringTest.cpp
===
klimek updated this revision to Diff 1.
klimek added a comment.
Fix bugs around marker replacements that neither insert nor delete anything.
https://reviews.llvm.org/D23119
Files:
lib/Tooling/Core/Replacement.cpp
unittests/Tooling/RefactoringTest.cpp
Index: unittests/Tooling/Refactorin
Author: klimek
Date: Wed Aug 3 09:12:17 2016
New Revision: 277597
URL: http://llvm.org/viewvc/llvm-project?rev=277597&view=rev
Log:
Fix quadratic runtime when adding items to tooling::Replacements.
Previously, we would search through all replacements when inserting a
new one to check for overlap
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277597: Fix quadratic runtime when adding items to
tooling::Replacements. (authored by klimek).
Changed prior to commit:
https://reviews.llvm.org/D23119?vs=1&id=3#toc
Repository:
rL LLVM
htt
klimek added inline comments.
Comment at: cfe/trunk/lib/Tooling/Core/Replacement.cpp:169
@@ +168,3 @@
+ // starts after R is (I+1).
+ if (I != Replaces.end() && *I == R)
+++I;
ioeric wrote:
> I think we should ignore replacement text when checking equality b
Author: klimek
Date: Wed Aug 3 10:12:00 2016
New Revision: 277603
URL: http://llvm.org/viewvc/llvm-project?rev=277603&view=rev
Log:
Fix bug in conflict check for Replacements::add().
We would not detect conflicts when inserting insertions at the same
offset as previously contained replacements.
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D23153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
klimek added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2809
@@ +2808,3 @@
+/// matches \c foo in \c foo(t);
+AST_MATCHER_P(OverloadExpr, canReferToDecl, internal::Matcher,
+ InnerMatcher) {
aaron.ballman wrote:
> I find th
klimek added a comment.
In https://reviews.llvm.org/D23279#509568, @omtcyfz wrote:
> And actually it makes much more sense for C than for C++. In C++ you just do
> `s/struct/class/g`, insert `public:` and you're golden.
That doesn't work if you want to minimize object size, though?
Repositor
klimek added a comment.
In https://reviews.llvm.org/D23279#509606, @omtcyfz wrote:
> In https://reviews.llvm.org/D23279#509567, @omtcyfz wrote:
>
> > In https://reviews.llvm.org/D23279#509047, @Eugene.Zelenko wrote:
> >
> > > May be this could be Clang-rename mode?
> >
> >
> > Definitely not.
> >
klimek added inline comments.
Comment at: include/clang/AST/RecursiveASTVisitor.h:892
@@ -891,1 +891,3 @@
+ else
+TRY_TO(TraverseStmt(LE->capture_init_begin()[C - LE->capture_begin()]));
return true;
I'd rather pass in the offset than doing math with what'
On Sat, Aug 13, 2016, 10:17 PM Zachary Turner wrote:
> The json is generated by CMake, so I don't thinks we can do this without
> patching CMake, which is a non-starter.
>
Why? We did add compilation database support to cmake in the first place.
Back then I did not support windows, btw, so I'd b
For years nobody worked on Windows support, so I'm somewhat surprised this
is suddenly time critical.
Usually the way this works is that we add the feature to upstream cmake,
and then make a recent enough cmake a requirement for tooling. There's no
need to make it a requirement for anything else. T
On Sun, Aug 14, 2016, 9:52 AM Zachary Turner wrote:
> I'll try and figure out who that was on Monday and loop them in. I'm not
> sure what problems the previous person ran into, but the test suite passes,
> i can run it on a large codebase, and all the results look fine and as if
> the tool is wo
I believe you forgot to add cfe-commits as subscriber on the patch.
On Sun, Feb 7, 2016 at 12:51 PM Alexander Shukaev <
cl...@alexander.shukaev.name> wrote:
> On 12/14/2015 10:03 PM, Alexander Shukaev wrote:
> > On 12/11/2015 04:40 PM, Daniel Jasper wrote:
> >> Please submit patches to clang-form
Author: klimek
Date: Tue Feb 9 04:59:21 2016
New Revision: 260218
URL: http://llvm.org/viewvc/llvm-project?rev=260218&view=rev
Log:
Add AST matcher reference to documentation directory when building HTML docs.
Modified:
cfe/trunk/docs/CMakeLists.txt
Modified: cfe/trunk/docs/CMakeLists.txt
U
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D16524
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek added a subscriber: klimek.
klimek added a comment.
I have fixed that in r260218 for LibASTMatcherReference (basically using the
same mechanism, but only copying the one file). Not sure which is the better
approach, I considered globbing, but my gut feeling decided against it :)
Definitel
On Tue, Feb 9, 2016 at 10:03 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: rsmith
> Date: Tue Feb 9 14:59:05 2016
> New Revision: 260277
>
> URL: http://llvm.org/viewvc/llvm-project?rev=260277&view=rev
> Log:
> Simplify and rename ASTMatchFinder's getCXXRecordDec
On Wed, Feb 10, 2016 at 10:04 AM Manuel Klimek wrote:
> On Tue, Feb 9, 2016 at 10:03 PM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Tue Feb 9 14:59:05 2016
>> New Revision: 260277
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=260277&view
klimek added inline comments.
Comment at: lib/ASTMatchers/ASTMatchersInternal.cpp:408
@@ +407,3 @@
+ PatternSet Patterns(Names);
+
+ llvm::SmallString<128> Scratch;
That empty line confuses me for some reason.
Comment at: lib/ASTMatchers/ASTMat
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Repository:
rL LLVM
http://reviews.llvm.org/D17376
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
Stumbling over this (much too late, of course), is this still a problem for
you?
On Thu, Nov 26, 2015 at 5:01 PM jean-yves desbree via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> I use clang 3.6.2 with qt creator 3.5.1 on windows 7 for parsing code in
> this IDE.
> It works well.
>
> Howev
klimek added a reviewer: djasper.
Comment at: lib/Format/AffectedRangeManager.cpp:103
@@ -102,1 +102,3 @@
AnnotatedLine *Line, const AnnotatedLine *PreviousLine) {
+ assert(Line && "does not contain any line");
+
Perhaps we should change this to take a Line&
This revision was automatically updated to reflect the committed changes.
Closed by commit rL267855: Add missing newline in clang-rename output.
(authored by klimek).
Changed prior to commit:
http://reviews.llvm.org/D18957?vs=53222&id=55380#toc
Repository:
rL LLVM
http://reviews.llvm.org/D1
Author: klimek
Date: Thu Apr 28 01:46:44 2016
New Revision: 267855
URL: http://llvm.org/viewvc/llvm-project?rev=267855&view=rev
Log:
Add missing newline in clang-rename output.
Patch by Miklos Vajna.
Differential Revision: http://reviews.llvm.org/D18957
Modified:
clang-tools-extra/trunk/cla
Author: klimek
Date: Thu Apr 28 08:37:45 2016
New Revision: 267877
URL: http://llvm.org/viewvc/llvm-project?rev=267877&view=rev
Log:
Fix spuriously dematerializing reference bug. Fixes PR26612.
Modified:
cfe/trunk/docs/CMakeLists.txt
Modified: cfe/trunk/docs/CMakeLists.txt
URL:
http://llvm.
Author: klimek
Date: Thu Apr 28 09:28:19 2016
New Revision: 267883
URL: http://llvm.org/viewvc/llvm-project?rev=267883&view=rev
Log:
Fix build.
Modified:
cfe/trunk/docs/CMakeLists.txt
Modified: cfe/trunk/docs/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/CMakeLists.
klimek added inline comments.
Comment at: include-fixer/InMemoryXrefsDB.cpp:18
@@ +17,3 @@
+InMemoryXrefsDB::InMemoryXrefsDB(
+std::map> LookupTable) {
+ for (const auto &Entry : LookupTable) {
I'd make that a const ref.
Comment at: include-
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:173-182
@@ -165,1 +172,12 @@
+void FindAllSymbols::addPragmaHeader(FileID ID, llvm::StringRef FilePath) {
+ PragmaHeaderMap[ID] = FilePath;
+}
+
+llvm::Optional FindAllSymbols::getPragma
klimek added inline comments.
Comment at: include-fixer/IncludeFixer.h:34-35
@@ -33,3 +33,4 @@
IncludeFixerActionFactory(
- XrefsDB &Xrefs, std::vector &Replacements,
+ XrefsDBManager &XrefsDBMgr,
+ std::vector &Replacements,
bool MinimizeIncludePaths = tr
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: include-fixer/InMemoryXrefsDB.cpp:24-26
@@ +23,5 @@
+for (const auto &Header : Entry.second) {
+ SymbolInfo Info;
+ Info.Name = Names.back();
+
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG. Thanks!
http://reviews.llvm.org/D19905
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
klimek added a comment.
(let me know if you need me to submit this)
http://reviews.llvm.org/D19905
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Note: if you intend to send more patches in the future, please consider
becoming one. It's very painless :)
On Wed, May 4, 2016 at 11:38 AM Miklos Vajna wrote:
> vmiklos added a comment.
>
> Yes, please submit it; I'm not a committer.
>
>
> http://reviews.llvm.org/D19905
>
>
>
>
klimek added a comment.
Note: if you intend to send more patches in the future, please consider
becoming one. It's very painless :)
http://reviews.llvm.org/D19905
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/
Author: klimek
Date: Wed May 4 04:45:44 2016
New Revision: 268484
URL: http://llvm.org/viewvc/llvm-project?rev=268484&view=rev
Log:
When renaming a class, ename pointers to that class as well.
Patch by Miklos Vajna.
Added:
clang-tools-extra/trunk/test/clang-rename/ClassTest.cpp
Modified:
klimek closed this revision.
klimek added a comment.
Submitted as r268484
http://reviews.llvm.org/D19905
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:48
@@ +47,3 @@
+ explicit FindAllSymbols(ResultReporter *Reporter,
+ HeaderMapCollector *Collector)
+ : Reporter(Reporter), Collector(Collector) {}
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/SymbolInfo.h:90-93
@@ +89,6 @@
+
+ static SymbolInfo
+ CreateFunctionSymbolInfo(const std::string &Name, const std::string
&FilePath,
+ const std::vector &Contexts, int
LineNumbe
klimek added inline comments.
Comment at: unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp:418
@@ +417,3 @@
+
+ {
+SymbolInfo Symbol =
Why are the other test cases using an extra block?
http://reviews.llvm.org/D19816
_
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/SymbolInfo.h:90-93
@@ +89,6 @@
+
+ static SymbolInfo
+ CreateFunctionSymbolInfo(const std::string &Name, const std::string
&FilePath,
+ const std::vector &Contexts, int
LineNumbe
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/SymbolInfo.h:98-101
@@ +97,6 @@
+
+ static SymbolInfo
+ CreateFunctionSymbolInfo(const std::string &Name, const std::string
&FilePath,
+ const std::vector &Contexts, int
LineNumb
klimek accepted this revision.
This revision is now accepted and ready to land.
Comment at: test/clang-rename/FieldTest.cpp:7-9
@@ +6,5 @@
+};
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=18 -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
--
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D20059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
Comment at: clang-rename/USRLocFinder.cpp:64
@@ +63,3 @@
+ if (Initializer->getSourceOrder() == -1) {
+// Ignore implicit initializers.
+continue;
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D20095
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: docs/include-fixer.rst:16
@@ +15,3 @@
+
+To use :program:`clang-include-fixer` two databases are required, both can be
+generated with existing tools.
-
On Thu, May 12, 2016 at 9:22 AM Miklos Vajna wrote:
> vmiklos added a comment.
>
> Hi,
>
> > Also: should we add a check that the token of the source location we
> find actually has the old name?
>
>
> Hmm, how do I get the token at a specific SourceLocation? The best I found
> so far is SourceMa
klimek added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:37
@@ -35,3 +36,3 @@
public:
- explicit USRLocFindingASTVisitor(const std::string USR) : USR(USR) {
+ explicit USRLocFindingASTVisitor(const std::string USR, const std::string
&PrevName) : USR(USR), PrevNa
klimek added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:75
@@ +74,3 @@
+ StringRef TokenName =
Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
Context.getSourceManager(), Context.getLangOpts());
+ if (TokenName.startswith(PrevName))
klimek added a comment.
We had problems with binary size in multiple places.
Overall, I think if we care about binary size we should try to make binary size
smaller at a higher level (perhaps by hiding visibility of symbols by default;
not sure how much that would affect the exe's though).
Unti
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: clang-rename/USRLocFinder.cpp:126-128
@@ -118,3 +125,5 @@
// All the locations of the USR were found.
- const std::string USR;
+ StringRef USR;
+ // Old n
klimek added inline comments.
Comment at: include/clang/Format/Format.h:855
@@ -854,1 +854,3 @@
+// \brief Returns a string representation of ``Language`` for debugging.
+inline StringRef getLanguageName(FormatStyle::LanguageKind Language) {
s/for debugging/.
Or
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG with nit to fix.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:44-45
@@ -41,2 +43,4 @@
- explicit FindAllSymbols(ResultReporter *Reporter) : Reporter(Repor
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D20296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D20356
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek added inline comments.
Comment at: lib/Format/SortJavaScriptImports.cpp:46-47
@@ +45,4 @@
+// An ES6 module reference.
+//
+// ES6 implements a module system, where individual modules (~= source files)
+// can reference other modules, either importing symbols from them, or
klimek added a comment.
We're getting there. Couple of nits left.
Comment at: lib/Format/SortJavaScriptImports.cpp:94-97
@@ +93,6 @@
+// Side effect imports might be ordering sensitive. Consider them equal so
+// that they maintain their relative order in the stable sort
klimek accepted this revision.
klimek added a comment.
lg
http://reviews.llvm.org/D20198
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek added inline comments.
Comment at: lib/Format/SortJavaScriptImports.cpp:160
@@ +159,3 @@
+ if (i + 1 < e) {
+// Insert breaks between imports.
+ReferencesText += "\n";
Between categories of imports and imports and exports, right?
=
klimek added inline comments.
Comment at: clang-rename/USRLocFinder.cpp:106-113
@@ -105,3 +105,10 @@
if (getUSRForDecl(Decl) == USR) {
- LocationsFound.push_back(Expr->getMemberLoc());
+ SourceLocation Location = Expr->getMemberLoc();
+ if (Location.isMacroID()
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D20446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek added inline comments.
Comment at: include/clang/Format/Format.h:769
@@ -768,1 +768,3 @@
+/// \brief Returns the replacements corresponding to applying \p Replaces and
+/// cleaning up the code after that.
cleanupAroundReplacements sounds good.
=
klimek added inline comments.
Comment at: lib/Format/Format.cpp:1450
@@ -1447,3 +1449,3 @@
public:
- Formatter(const FormatStyle &Style, SourceManager &SourceMgr, FileID ID,
-ArrayRef Ranges)
+ CodeProcessor(const FormatStyle &Style, SourceManager &SourceMgr, FileID
klimek added a subscriber: klimek.
Comment at: include-fixer/FixedXrefsDB.h:18
@@ +17,3 @@
+
+/// Xref database with fixed content, intended for testing
+// FIXME: Move to unittest once ClangIncludeFixer doesn't depend on it anymore.
Add '.', remove "intended for
klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
This revision is now accepted and ready to land.
Cool, lg.
Comment at: include-fixer/IncludeFixer.cpp:133
@@ +132,3 @@
+ StringRef filename() const { return Filename; }
+
+ /// Called for e
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D19356
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
Looks good.
http://reviews.llvm.org/D18957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
This revision is now accepted and ready to land.
We'll probably want Daniel to also take another look over it, as it's a pretty
substantial change that will haunt us for a while, but I think this now pretty
m
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D20537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
This revision is now accepted and ready to land.
LG. Thx!
http://reviews.llvm.org/D20767
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
klimek added a subscriber: klimek.
Comment at: include-fixer/IncludeFixer.cpp:397
@@ +396,3 @@
+ clang::tooling::Replacements Insertions;
+ if (FirstIncludeOffset == -1U) {
+// FIXME: skip header guards.
Do we want UINT_MAX? I find the wording in the standar
klimek added a reviewer: bkramer.
klimek added a comment.
Generally makes sense; adding d0k for additional thoughts.
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
klimek added inline comments.
Comment at: include-fixer/IncludeFixer.h:74
@@ +73,3 @@
+/// \return Replacements for inserting and sorting headers.
+std::vector createInsertHeaderReplacements(
+StringRef Code, StringRef FilePath, StringRef Header,
This is still
klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
lg
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
I think this is ok, mainly because the clean paths are explicitly for user
code, where users usually like to have, well, clean paths.
http://reviews.llvm.org/D20819
___
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D20635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek added inline comments.
Comment at: test/clang-rename/StaticCastExpr.cpp:4-25
@@ +3,24 @@
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+class Base
+{
+};
+
+class Derived : public Base
+{
+public:
+ int getValue() const
+ {
+ return 0;
+ }
+};
+
+int main()
+{
+ De
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D21012
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D21120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek added a comment.
Please add a test.
http://reviews.llvm.org/D21241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG, although I believe the recent history of changes indicates that the
approach is suboptimal, and we should address this on a higher level. But for
now fixing the bugs seems like the right s
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG. Fix the change description when submitting, though - this is not about an
enum comparison, right?
http://reviews.llvm.org/D21278
___
cfe-com
klimek added a subscriber: cfe-commits.
klimek added a comment.
Generally, please subscribe cfe-commits when sending patches via phab.
See http://llvm.org/docs/Phabricator.html
Repository:
rL LLVM
http://reviews.llvm.org/D21279
___
cfe-commits ma
klimek added a comment.
For 2), SourceLocation has getLocWithOffset?
http://reviews.llvm.org/D21364
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: clang-rename/USRLocFinder.cpp:94
@@ +93,3 @@
+ // that is not to be touched by the rename.
+
LocationsFound.push_back(DestructorDecl->getLocation().g
klimek added inline comments.
Comment at: clang-rename/USRFinder.h:28-33
@@ -27,6 +27,8 @@
-// Given an AST context and a point, returns a NamedDecl identifying the symbol
-// at the point. Returns null if nothing is found at the point.
+// Given an AST context and a point (or f
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D21517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
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
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
klimek added a comment.
Why don't we output replacements and use clang-apply-replacements?
http://reviews.llvm.org/D21676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Repository:
rL LLVM
http://reviews.llvm.org/D21677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
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
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";
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
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
http://reviews.llvm.org/D21676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
401 - 500 of 588 matches
Mail list logo
