Re: [PATCH] -include option and -exclude ignore arguments finishing with '/'

2013-07-30 Thread Edwin Vane
Didn't we discuss a precondition that paths provided to `isFileIncluded()` must not have relative operators in them? Comment at: unittests/cpp11-migrate/IncludeExcludeTest.cpp:66 @@ +65,3 @@ + EXPECT_TRUE(IEManager.isFileIncluded("c/c2/c3/f.cpp")); +} + Coul

Re: [PATCH] -include option and -exclude ignore arguments finishing with '/'

2013-07-30 Thread Edwin Vane
I didn't mean you actually change code. I mean to just document the fact that the path to isFileIncluded() must be absolute and contain no relative paths. http://llvm-reviews.chandlerc.com/D1134 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu ht

Re: [PATCH] -include option and -exclude ignore arguments finishing with '/'

2013-07-30 Thread Edwin Vane
Comment at: unittests/cpp11-migrate/IncludeExcludeTest.cpp:58 @@ +57,3 @@ + llvm::error_code Err = IEManager.readListFromString( + /*include=*/ "a/.,b/b2/,c/c2/c3/../../c2/,b/b2/./b3/,/b/b2/.", + /*exclude=*/ ""); I think this test would be better if

Re: [PATCH] -include option and -exclude ignore arguments finishing with '/'

2013-07-30 Thread Edwin Vane
LGTM http://llvm-reviews.chandlerc.com/D1134 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications

2013-08-07 Thread Edwin Vane
With respect to clang-tidy, should this code live in clang::tooling? Comment at: cpp11-migrate/Core/IncludeDirectives.h:11 @@ +10,3 @@ +/// \file +/// \brief This file declares the IncludeDirectives class that helps detecting +/// and modifying \#include directives. --

[PATCH] Introduce Replacement deduplication and conflict detection function

2013-08-07 Thread Edwin Vane
Hi klimek, djasper, silvas, This patch adds tooling::deduplicate() which removes duplicates from and looks for conflicts in a vector of Replacements. Questions for reviewers: * Use std::pair instead of Range to describe the Replacements involved in a conflict? .first and .second would be offs

Re: [PATCH] Introduce Replacement deduplication and conflict detection function

2013-08-08 Thread Edwin Vane
1. I was concerned that although `Range` describes what I want that my use of it to describe an offset and number of consecutive items in a vector was against it's intended use as a **source** range. 2. Just making sure. When we switch `Replacements` to be an `std::vector` from `std::set` w

Re: [PATCH] Introduce Replacement deduplication and conflict detection function

2013-08-08 Thread Edwin Vane
Err. nvm, that last question http://llvm-reviews.chandlerc.com/D1314 BRANCH dedup ARCANIST PROJECT clang ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Re: [PATCH] Introduce Replacement deduplication and conflict detection function

2013-08-08 Thread Edwin Vane
Closed by commit rL187979 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1314?vs=3256&id=3283#toc http://llvm-reviews.chandlerc.com/D1314 COMMIT http://llvm-reviews.chandlerc.com/rL187979 ___ cfe-commits mailin

r187979 - Introduce Replacement deduplication and conflict detection function

2013-08-08 Thread Edwin Vane
Author: revane Date: Thu Aug 8 08:31:14 2013 New Revision: 187979 URL: http://llvm.org/viewvc/llvm-project?rev=187979&view=rev Log: Introduce Replacement deduplication and conflict detection function Summary: This patch adds tooling::deduplicate() which removes duplicates from and looks for conf

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications

2013-08-09 Thread Edwin Vane
So do you have solutions for the problem of headers with header guards but no includes? I'm fine documenting the //X Macro header// problem as a known shortcoming for now. I'd place a FIXME somewhere if there's an appropriate spot. Also log a bug in JIRA. How easy is it to detect if a head

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-09 Thread Edwin Vane
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:247-249 @@ +246,5 @@ +} +if (!Compilations) { + llvm::report_fatal_error(ErrorMessage); +} + } Single-line if statement has no braces. http://llvm-reviews.chandlerc.com/D1337

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications

2013-08-09 Thread Edwin Vane
ivesTest.cpp:165-167 @@ +164,5 @@ +TEST(IncludeDirectivesTest, indirectIncludes) { + EXPECT_EQ( + "#include \n", + addIncludeInCode("/virtual/foo-inner.h", "#include \n")); + ---- Guillaume Papin wrote: > Edwin Vane wrote: > > I'm still n

Re: [PATCH] cpp11-migrate: Write header replacements to disk

2013-08-12 Thread Edwin Vane
Comment at: clang-tools-extra/trunk/cpp11-migrate/Core/ReplacementsYaml.h:27 @@ +26,3 @@ + std::string TransformID; + std::vector GeneratedReplacements; +}; GeneratedReplacements -> Replacements Comment at: clang-tools-extra/trunk/cpp11-migrat

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-13 Thread Edwin Vane
I agree. If `-p` is used, even incorrectly, we shouldn't do anything with adding `-std=c++11` at all. If `--` is used explicitly it should take precedence and we definitely do not add `-std=c++11` which is already happening. If `-p` is used, we know a value is required but if it's the empty

[PATCH] Adding a vector version of tooling::applyAllReplacements

2013-08-13 Thread Edwin Vane
Hi klimek, One day soon, tooling::Replacements will be changed from being implemented as an std::set to being implemented as an std::vector. Until then, some new code using vectors of Replacements would enjoy having a version of applyAllReplacements that takes a vector. http://llvm-reviews.chandl

r188287 - Fixing a conflict detection bug in tooling::deduplicate

2013-08-13 Thread Edwin Vane
Author: revane Date: Tue Aug 13 11:26:44 2013 New Revision: 188287 URL: http://llvm.org/viewvc/llvm-project?rev=188287&view=rev Log: Fixing a conflict detection bug in tooling::deduplicate If a Replacment is contained within the conflict range being built, the conflict range would be erroneously

Re: [PATCH] Fixes test failure on darwin introduced by r188274

2013-08-13 Thread Edwin Vane
Comment at: test/cpp11-migrate/HeaderReplacements/main.cpp:11-13 @@ -10,5 +10,5 @@ // RUN: ls -1 %t/Test | grep -c "common.cpp_common.h_.*.yaml" | grep "^1$" -// We need to remove the path from FileNames in the generated YAML file because it will have a path in the temp directo

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-13 Thread Edwin Vane
The documentation should be updated to reflect this new behaviour. It should be explicitly spelled out instead of referring to other documentation. Should talk about the priority of input: # If `--` is used, the args after, if any, are used # If `-p` is used, that compilation database is

Re: [PATCH] Fixes test failure on darwin introduced by r188274

2013-08-13 Thread Edwin Vane
Comment at: test/cpp11-migrate/HeaderReplacements/main.cpp:14 @@ -14,1 +13,3 @@ +// RUN: sed -e 's#$(path)#%t/Test#g' %S/common.h.yaml > %t/Test/main.cpp_common.h.yaml +// RUN: sed -i -e 's#\\#/#g' %t/Test/main.cpp_common.h_*.yaml // RUN: diff -b %t/Test/main.cpp_common.h.yaml

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-13 Thread Edwin Vane
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:237-238 @@ +236,4 @@ +} else { + Compilations.reset(CompilationDatabase::autoDetectFromSource( + SourcePaths[0], ErrorMessage)); + // If no compilation database can be detected from source then we create -

r188295 - Adding a vector version of tooling::applyAllReplacements

2013-08-13 Thread Edwin Vane
Author: revane Date: Tue Aug 13 12:38:19 2013 New Revision: 188295 URL: http://llvm.org/viewvc/llvm-project?rev=188295&view=rev Log: Adding a vector version of tooling::applyAllReplacements One day soon, tooling::Replacements will be changed from being implemented as an std::set to being implemen

Re: [PATCH] Adding a vector version of tooling::applyAllReplacements

2013-08-13 Thread Edwin Vane
Closed by commit rL188295 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1380?vs=3432&id=3435#toc http://llvm-reviews.chandlerc.com/D1380 COMMIT http://llvm-reviews.chandlerc.com/rL188295 ___ cfe-commits mailin

[PATCH] cpp11-migrate: Header change application tool - migmerge

2013-08-13 Thread Edwin Vane
Hi tareqsiraj, arielbernal, Sarcasm, klimek, Introducing new tool 'migmerge' to merge and apply changes to headers as found in header change description files generated by the C++11 Migrator. CMake files updated to build new tool. Includes a conflict test case. http://llvm-reviews.chandlerc.com

r188304 - Have Range::overlapsWith use positive logic

2013-08-13 Thread Edwin Vane
Author: revane Date: Tue Aug 13 13:11:16 2013 New Revision: 188304 URL: http://llvm.org/viewvc/llvm-project?rev=188304&view=rev Log: Have Range::overlapsWith use positive logic Improved test to catch missing case. Modified: cfe/trunk/include/clang/Tooling/Refactoring.h cfe/trunk/unittes

[PATCH] cpp11-migrate: Adding -yaml-only option

2013-08-13 Thread Edwin Vane
Hi tareqsiraj, arielbernal, Sarcasm, For use with -headers, -yaml-only will cause cpp11-migrate to not write header changes to disk and instead write them as header change description files. This option facilitiates upcoming functionality to properly support changing headers as part of migration.

[clang-tools-extra] r188371 - cpp11-migrate: Adding -yaml-only option

2013-08-14 Thread Edwin Vane
Author: revane Date: Wed Aug 14 09:52:45 2013 New Revision: 188371 URL: http://llvm.org/viewvc/llvm-project?rev=188371&view=rev Log: cpp11-migrate: Adding -yaml-only option For use with -headers, -yaml-only will cause cpp11-migrate to not write header changes to disk and instead write them as hea

Re: [PATCH] cpp11-migrate: Adding -yaml-only option

2013-08-14 Thread Edwin Vane
Closed by commit rL188371 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1385?vs=3443&id=3467#toc http://llvm-reviews.chandlerc.com/D1385 COMMIT http://llvm-reviews.chandlerc.com/rL188371 ___ cfe-commits mailin

[clang-tools-extra] r188374 - cpp11-migrate: Fix silly logic error preventing multiple transforms

2013-08-14 Thread Edwin Vane
Author: revane Date: Wed Aug 14 10:09:44 2013 New Revision: 188374 URL: http://llvm.org/viewvc/llvm-project?rev=188374&view=rev Log: cpp11-migrate: Fix silly logic error preventing multiple transforms A missed clause in an error test added in r188371 caused any use of the migrator requesting mult

Re: [PATCH] cpp11-migrate: Header change application tool - migmerge

2013-08-14 Thread Edwin Vane
@Sarcasm: The .git thing is actually left over from when I copied this code out of [[https://github.com/revane/migmerge_git|migmerge_git]]. Now that you remind me of this fact, I think this tool needs some support for knowing which directories to look in and which to avoid. For now I think

[PATCH] cpp11-migrate: Remove mention of 'headers' from serialization code

2013-08-14 Thread Edwin Vane
Hi tareqsiraj, arielbernal, Sarcasm, * HeaderChangeDocument -> MigratorDocument * HeaderFileName -> TargetFile * SourceFileName -> MainSourceFile * Removed TransformID * Comments updated, at least with respect to serialization * Unit tests updated. http://llvm-reviews.chandlerc.com/D1403 Files:

Re: [PATCH] cpp11-migrate: Remove mention of 'headers' from serialization code

2013-08-14 Thread Edwin Vane
Closed by commit rL188404 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1403?vs=3473&id=3477#toc http://llvm-reviews.chandlerc.com/D1403 COMMIT http://llvm-reviews.chandlerc.com/rL188404 ___ cfe-commits mailin

[clang-tools-extra] r188404 - cpp11-migrate: Remove mention of 'headers' from serialization code

2013-08-14 Thread Edwin Vane
Author: revane Date: Wed Aug 14 14:02:28 2013 New Revision: 188404 URL: http://llvm.org/viewvc/llvm-project?rev=188404&view=rev Log: cpp11-migrate: Remove mention of 'headers' from serialization code * HeaderChangeDocument -> MigratorDocument * HeaderFileName -> TargetFile * SourceFileName -> Mai

[PATCH] cpp11-migrate: Further generalize replacements serialization

2013-08-15 Thread Edwin Vane
Hi klimek, sdt, tareqsiraj, arielbernal, Sarcasm, In preparation for a move into clang::tooling and use in a more general Replacement application tool, the Replacement serialization is further generalized: * TargetFile now serialized with every replacement lifting the restriction that serialized

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-15 Thread Edwin Vane
Few rewordings. Comment at: docs/MigratorUsage.rst:34-40 @@ -33,9 +33,9 @@ is the directory containing a file named ``compile_commands.json`` which provides compiler arguments for building each source file. CMake can generate this file by specifying ``-DCMAK

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-15 Thread Edwin Vane
One more fix. Comment at: docs/cpp11-migrate.rst:44 @@ -43,3 +43,3 @@ arguments, it might be easier to pass the compiler args on the command line -after ``--``. If you're working with multiple files or even a single file -with many compiler args, it's probably best to use a *

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications

2013-08-15 Thread Edwin Vane
Just a few small things from me. I would remove mention of what you call X Macro Headers and just say "headers meant for multiple inclusion". Otherwise, the rest of the code, albeit complex, seems ok. I couldn't see any tests that were obviously missing. Comment at: cp

Re: [PATCH] cpp11-migrate: Further generalize replacements serialization

2013-08-15 Thread Edwin Vane
- Reviewer changes Hi klimek, sdt, tareqsiraj, arielbernal, Sarcasm, http://llvm-reviews.chandlerc.com/D1413 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1413?vs=3499&id=3502#toc Files: cpp11-migrate/Core/FileOverrides.cpp cpp11-migrate/Core/FileOverrides.h cpp11-migrat

[PATCH] Templatize tooling::deduplicate()

2013-08-15 Thread Edwin Vane
Hi klimek, silvas, djasper, To facilitate deduplication of Replacements with extra data attached, deduplicate() has been generalized to work on vectors of type T. tooling::Replacement already satisfies all requirements of type T so no changes to existing code are necessary. http://llvm-reviews.ch

Re: [PATCH] Use -std=c++11 when no arguments are provided.

2013-08-15 Thread Edwin Vane
One small fix. Otherwise LGTM. Comment at: docs/MigratorUsage.rst:49-50 @@ +48,4 @@ + + If *neither* ``--`` *nor* ``-p`` are specified a compilation database is + searched **for** starting with the path of the first-provided source file and + proceeding through parent direc

Re: [PATCH] Templatize tooling::deduplicate()

2013-08-15 Thread Edwin Vane
- Fix Hi klimek, silvas, djasper, http://llvm-reviews.chandlerc.com/D1415 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1415?vs=3506&id=3509#toc Files: include/clang/Tooling/Refactoring.h lib/Tooling/Refactoring.cpp Index: include/clang/Tooling/Refactoring.h =

Re: [PATCH] Templatize tooling::deduplicate()

2013-08-15 Thread Edwin Vane
I considered using a map off to the side keyed on replacements but it seemed like such monstrous overhead when the obvious thing to do was to deduplicate the objects in place. As @klimek is fond of reminding me, in MLOC code bases, the collection of replacements can be huge. Duplicating it ju

Re: [PATCH] Templatize tooling::deduplicate()

2013-08-16 Thread Edwin Vane
I do actually have a case for this. The replacement applicator tool, D1382, is being turned into a more general-purpose applicator called `clang-replace`. See my response to @silvas for an explanation of that case. However, I submitted this patch in support of my work on clang-replace befor

Re: [PATCH] Templatize tooling::deduplicate()

2013-08-16 Thread Edwin Vane
Thinking about future directions some more I think I'll forget about Context for now and forget about the templatized deduplicate. I will however fix the comparison order and keep operator== and operator<. http://llvm-reviews.chandlerc.com/D1415 ___

r188550 - Tweak Replacement comparisons

2013-08-16 Thread Edwin Vane
Author: revane Date: Fri Aug 16 07:18:53 2013 New Revision: 188550 URL: http://llvm.org/viewvc/llvm-project?rev=188550&view=rev Log: Tweak Replacement comparisons * Introduce operator< to replace Replacement::Less * Make operator== and operator< on Replacements non-member functions * Change order

[PATCH] Adding Replacement serialization support

2013-08-16 Thread Edwin Vane
Hi klimek, djasper, silvas, Adding a YAML document definition for serializing a vector of Replacements. Document is basically just a sequence of Replacements but has an optional 'Context' field a producer may provide to annotate the Replacements serialized in a single document. Tests added. http

[PATCH] cpp11-migrate: Use Replacement serialization from clang::tooling

2013-08-16 Thread Edwin Vane
Hi tareqsiraj, arielbernal, Sarcasm, Serialization of replacements has been moved to clang::tooling. Note to reviewers: This patch depends on D1422 and replaces D1413. http://llvm-reviews.chandlerc.com/D1423 Files: cpp11-migrate/Core/FileOverrides.cpp cpp11-migrate/Core/FileOverrides.h cp

Re: [PATCH] cpp11-migrate: Further generalize replacements serialization

2013-08-16 Thread Edwin Vane
Replaced by D1423 and D1422. http://llvm-reviews.chandlerc.com/D1413 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

[PATCH] Introducing new tool clang-replace

2013-08-16 Thread Edwin Vane
Hi klimek, djasper, silvas, tareqsiraj, arielbernal, Sarcasm, Introducing new tool 'clang-replace' that finds files containing serialized Replacements and applies those changes after deduplication and detecting conflicts. Currently the tool does not apply changes. It stops just after the deduplic

Re: [PATCH] cpp11-migrate: Header change application tool - migmerge

2013-08-16 Thread Edwin Vane
Massive reworking of this patch has led to D1424. http://llvm-reviews.chandlerc.com/D1382 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Re: [PATCH] Adding Replacement serialization support

2013-08-16 Thread Edwin Vane
Comment at: include/clang/Tooling/ReplacementsYaml.h:28 @@ +27,3 @@ +/// \brief The top-level YAML document that contains all Replacements. +struct ReplacementsDocument { + /// A freeform chunk of text to describe the context of the replacements in Manuel Klimek

Re: [PATCH] Adding Replacement serialization support

2013-08-16 Thread Edwin Vane
- Ran clang-format yet again. Addressed some reviewer comments. Hi klimek, djasper, silvas, http://llvm-reviews.chandlerc.com/D1422 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1422?vs=3524&id=3534#toc Files: include/clang/Tooling/ReplacementsYaml.h unittests/Tooling/CMak

Re: [PATCH] Adding Replacement serialization support

2013-08-19 Thread Edwin Vane
Reviewer changes: - Moved ReplacementsDocument to Refactoring.h and renamed it TranslationUnitReplacements. - Added MainSourceFile field. - Improved test cases. Hi klimek, djasper, silvas, http://llvm-reviews.chandlerc.com/D1422 CHANGE SINCE LAST DIFF http://llvm-reviews.c

Re: [PATCH] Introducing new tool clang-replace

2013-08-19 Thread Edwin Vane
- clang-replace: Separate out replacement file finding - clang-replace: Update LIT test for windows Hi klimek, djasper, silvas, tareqsiraj, arielbernal, Sarcasm, http://llvm-reviews.chandlerc.com/D1424 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1424?vs=3526&id=3577#toc

Re: [PATCH] Introducing new tool clang-replace

2013-08-19 Thread Edwin Vane
- Update clang-replace to use new replacement serialization structures Hi klimek, djasper, silvas, tareqsiraj, arielbernal, Sarcasm, http://llvm-reviews.chandlerc.com/D1424 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1424?vs=3577&id=3581#toc Files: CMakeLists.txt Makefil

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications (Windows bot fix)

2013-08-19 Thread Edwin Vane
Not sure if this will work without you trying it first. It was a while back and perhaps there are better ways of doing it but I fixed these OS-specific file path problems in a way shown in the unittest Transform.isFileModifiable(). http://llvm-reviews.chandlerc.com/D1438 _

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications (Windows bot fix)

2013-08-20 Thread Edwin Vane
I guess we won't know for sure unless you try it on Windows. I'm guessing you don't have a windows machine? If not, I guess the only option is to try it on the buildbots. http://llvm-reviews.chandlerc.com/D1438 BRANCH include-directives ARCANIST PROJECT clang-tools-extra ___

Re: [PATCH] Adding Replacement serialization support

2013-08-20 Thread Edwin Vane
Closed by commit rL188818 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1422?vs=3570&id=3604#toc http://llvm-reviews.chandlerc.com/D1422 COMMIT http://llvm-reviews.chandlerc.com/rL188818 ___ cfe-commits mailin

r188818 - Adding Replacement serialization support

2013-08-20 Thread Edwin Vane
Author: revane Date: Tue Aug 20 14:07:21 2013 New Revision: 188818 URL: http://llvm.org/viewvc/llvm-project?rev=188818&view=rev Log: Adding Replacement serialization support Adding a new data structure for storing the Replacements generated for a single translation unit. Structure contains a vect

Re: [PATCH] cpp11-migrate: Use Replacement serialization from clang::tooling

2013-08-20 Thread Edwin Vane
Closed by commit rL188820 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1423?vs=3525&id=3605#toc http://llvm-reviews.chandlerc.com/D1423 COMMIT http://llvm-reviews.chandlerc.com/rL188820 ___ cfe-commits mailin

[clang-tools-extra] r188820 - cpp11-migrate: Use Replacement serialization from clang::tooling

2013-08-20 Thread Edwin Vane
Author: revane Date: Tue Aug 20 14:20:52 2013 New Revision: 188820 URL: http://llvm.org/viewvc/llvm-project?rev=188820&view=rev Log: cpp11-migrate: Use Replacement serialization from clang::tooling Serialization of replacements has been moved to clang::tooling. Differential Revision: http://llvm

Re: [PATCH] Introducing new tool clang-replace

2013-08-21 Thread Edwin Vane
Reviewer changes and other polish: * Diagnostics not passed as an argument where SourceManager is. * Renamed applyReplacements to better indicate what it does. * Introduced the clang::replace namespace. * Various renaming of variables and updating of docs to not mention 'do

Re: [PATCH] Introducing new tool clang-replace

2013-08-21 Thread Edwin Vane
- Restoring proper file mode Hi klimek, djasper, silvas, tareqsiraj, arielbernal, Sarcasm, http://llvm-reviews.chandlerc.com/D1424 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1424?vs=3628&id=3629#toc Files: CMakeLists.txt Makefile clang-replace/ApplyReplacements.cpp

Re: [PATCH] cpp11-migrate: Add a class to support include directives modifications (Windows bot fix)

2013-08-21 Thread Edwin Vane
Comment at: unittests/cpp11-migrate/IncludeDirectivesTest.cpp:25 @@ +24,3 @@ + llvm::error_code EC = llvm::sys::fs::current_path(CurrentDir); + assert(!EC); + (void)EC; Instead of using assertions, how about using `ASSERT_*` and then wrapping calls to `applyA

Re: [PATCH] cpp11-migrate: Add a transform that use pass-by-value in constructors

2013-08-21 Thread Edwin Vane
Comment at: cpp11-migrate/PassByValue/PassByValue.h:25 @@ +24,3 @@ +/// \brief Subclass of Transform that uses pass-by-value semantic where +/// applicable. +/// I'd be a bit more specific in this one-line summary and replace `applicable` with `when move constru

[PATCH] clang-replace: Write merged changes to disk

2013-08-21 Thread Edwin Vane
Hi klimek, djasper, Functionality for clang-replace completed with the addition of the ability to write merged replacements to disk. Test added. http://llvm-reviews.chandlerc.com/D1460 Files: clang-replace/ApplyReplacements.cpp clang-replace/ApplyReplacements.h clang-replace/tool/ClangRep

Re: [PATCH] clang-replace: Write merged changes to disk

2013-08-22 Thread Edwin Vane
Comment at: clang-replace/ApplyReplacements.cpp:210 @@ +209,3 @@ +if (!ErrorInfo.empty()) { + errs() << "Unable to open " << FileName << " for writing\n"; + return false; Manuel Klimek wrote: > Since we're already using Diagnostics for everything el

[clang-tools-extra] r189008 - Introducing new tool clang-replace

2013-08-22 Thread Edwin Vane
Author: revane Date: Thu Aug 22 08:07:14 2013 New Revision: 189008 URL: http://llvm.org/viewvc/llvm-project?rev=189008&view=rev Log: Introducing new tool clang-replace Introducing new tool 'clang-replace' that finds files containing serialized Replacements and applies those changes after deduplic

Re: [PATCH] Introducing new tool clang-replace

2013-08-22 Thread Edwin Vane
Closed by commit rL189008 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1424?vs=3629&id=3668#toc http://llvm-reviews.chandlerc.com/D1424 COMMIT http://llvm-reviews.chandlerc.com/rL189008 ___ cfe-commits mailin

Re: [PATCH] clang-replace: Write merged changes to disk

2013-08-22 Thread Edwin Vane
Comment at: clang-replace/ApplyReplacements.cpp:210 @@ +209,3 @@ +if (!ErrorInfo.empty()) { + errs() << "Unable to open " << FileName << " for writing\n"; + return false; Edwin Vane wrote: > Manuel Kl

Re: [PATCH] clang-replace: Write merged changes to disk

2013-08-22 Thread Edwin Vane
Closed by commit rL189014 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1460?vs=3634&id=3670#toc http://llvm-reviews.chandlerc.com/D1460 COMMIT http://llvm-reviews.chandlerc.com/rL189014 ___ cfe-commits mailin

[clang-tools-extra] r189014 - clang-replace: Write merged changes to disk

2013-08-22 Thread Edwin Vane
Author: revane Date: Thu Aug 22 08:40:32 2013 New Revision: 189014 URL: http://llvm.org/viewvc/llvm-project?rev=189014&view=rev Log: clang-replace: Write merged changes to disk Functionality for clang-replace completed with the addition of the ability to write merged replacements to disk. Test a

[clang-tools-extra] r189019 - clang-replace: Exclude test-time deps from LIT test suite

2013-08-22 Thread Edwin Vane
Author: revane Date: Thu Aug 22 10:04:34 2013 New Revision: 189019 URL: http://llvm.org/viewvc/llvm-project?rev=189019&view=rev Log: clang-replace: Exclude test-time deps from LIT test suite Subdirectories of test/clang-replace contain test-time dependencies and not more LIT tests. Tell LIT to ig

Re: [PATCH] cpp11-migrate: Add a transform that use pass-by-value in constructors

2013-08-22 Thread Edwin Vane
Comment at: cpp11-migrate/PassByValue/PassByValueActions.cpp:77 @@ +76,3 @@ + +/// \brief Find all references to \p ParmVarDecls accross all of the +/// constructors redeclarations. Guillaume Papin wrote: > Edwin Vane wrote: > > Typo `ParmVarDecls` ->

[clang-tools-extra] r189047 - clang-replace: Layout of test directory now more standard

2013-08-22 Thread Edwin Vane
Author: revane Date: Thu Aug 22 14:44:07 2013 New Revision: 189047 URL: http://llvm.org/viewvc/llvm-project?rev=189047&view=rev Log: clang-replace: Layout of test directory now more standard Test-time dependencies now live within test/clang-replace/Inputs which is more in line with llvm and clang

Re: [PATCH] clang-replace: Delete change description files

2013-08-26 Thread Edwin Vane
Comment at: clang-replace/tool/ClangReplaceMain.cpp:28 @@ -27,1 +27,3 @@ +static cl::opt RemoveTUReplacementFiles( +"remove-change-desc-files", I thought the point was to remove files only if everything *was* successfully applied? Default behaviour should

Re: [PATCH] clang-replace: Delete change description files

2013-08-26 Thread Edwin Vane
Comment at: clang-replace/tool/ClangReplaceMain.cpp:28 @@ -27,1 +27,3 @@ +static cl::opt RemoveTUReplacementFiles( +"remove-change-desc-files", Tareq A. Siraj wrote: > Edwin Vane wrote: > > I thought the point was to remove files only

[PATCH] clang-replace: Afford applying replacements in memory

2013-08-26 Thread Edwin Vane
Hi klimek, For users of libclangReplace, this patch affords the ability to apply replacements in memory instead of writing to disk. http://llvm-reviews.chandlerc.com/D1519 Files: clang-replace/ApplyReplacements.cpp clang-replace/ApplyReplacements.h Index: clang-replace/ApplyReplacements.cpp

Re: [PATCH] clang-replace: Afford applying replacements in memory

2013-08-26 Thread Edwin Vane
Comment at: clang-replace/ApplyReplacements.h:87 @@ -84,1 +86,3 @@ + clang::SourceManager &SM, + llvm::StringMap *OutputState = 0); Manuel Klimek wrote: > I'd rather split up this function into one that does the apply

Re: [PATCH] clang-replace: Delete change description files

2013-08-26 Thread Edwin Vane
Comment at: clang-replace/ApplyReplacements.cpp:224 @@ -220,1 +223,3 @@ +bool deleteReplacementFiles(const TUReplacementFiles &Files) { + bool Success = true; This function should take Diagnostics. Don't have to use it yet. http://llvm-reviews.chandlerc.com/

Re: [PATCH] clang-replace: Afford applying replacements in memory

2013-08-26 Thread Edwin Vane
- Switching back to two-function apply-and-write logic Hi klimek, http://llvm-reviews.chandlerc.com/D1519 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1519?vs=3761&id=3765#toc Files: clang-replace/ApplyReplacements.cpp clang-replace/ApplyReplacements.h clang-replace/too

[PATCH] clang-replace: Re-org of file structure

2013-08-26 Thread Edwin Vane
Hi klimek, Creating an include and lib directory for sources. Updating build systems to match. This reorg will make it slightly nicer for users of libclangReplace. Depends on D1519. http://llvm-reviews.chandlerc.com/D1522 Files: clang-replace/CMakeLists.txt clang-replace/Makefile clang-re

Re: [PATCH] clang-replace: Re-org of file structure

2013-08-27 Thread Edwin Vane
Eventual inclusion into clang itself was a reason why I wanted to re-org the directories to mirror clang. Thinking about it overnight I think there needs to be an extra directory here: include/clang-replace/Replace/ instead of include/Replace. clang-tools-extra seemed like a good place to inc

r189344 - Adding const buffer iterator generators to Rewriter

2013-08-27 Thread Edwin Vane
Author: revane Date: Tue Aug 27 08:00:34 2013 New Revision: 189344 URL: http://llvm.org/viewvc/llvm-project?rev=189344&view=rev Log: Adding const buffer iterator generators to Rewriter Modified: cfe/trunk/include/clang/Rewrite/Core/Rewriter.h Modified: cfe/trunk/include/clang/Rewrite/Core/R

Re: [PATCH] clang-replace: Afford applying replacements in memory

2013-08-27 Thread Edwin Vane
- comment fix - Using Rewriter to transfer data between applyReplacements and writeFiles Hi klimek, http://llvm-reviews.chandlerc.com/D1519 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1519?vs=3765&id=3808#toc Files: clang-replace/ApplyReplacements.cpp clang-replace/A

Re: [PATCH] clang-replace: Re-org of file structure

2013-08-27 Thread Edwin Vane
- Adding clang-replace subdirectory to 'include' Hi klimek, chandlerc, http://llvm-reviews.chandlerc.com/D1522 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1522?vs=3768&id=3809#toc Files: clang-replace/CMakeLists.txt clang-replace/Makefile clang-replace/include/clang-re

Re: [PATCH] clang-replace: Re-org of file structure

2013-08-27 Thread Edwin Vane
Some changes are starting to leak from D1519 due to a rebase. All that changed here was the introduction of the `clang-replace` subdirectory of `include` and changes to `#include` directives where the header was included. http://llvm-reviews.chandlerc.com/D1522 ___

Re: [PATCH] Transform all files in a compilation database if no source files are provided.

2013-08-27 Thread Edwin Vane
Main problem I see here is that if you don't explicitly include a directory, no files get transformed. We should by default transform every file in the compilation database. Since you're using the `IncludingExcludeInfo` structure to identify modifiable sources and headers (which is fine I thi

Re: [PATCH] cpp11-migrate: Add a transform that use pass-by-value in constructors

2013-08-27 Thread Edwin Vane
Don't worry about the blurb on templates now. Now that the behaviour with args that are not template args has been found to work fine no need to talk about the case that obviously shouldn't work. One, hopefully last, request: can you add a test where the constructor has multiple args, >1 t

r189358 - Adding a vector version of clang::tooling::shiftedCodePosition().

2013-08-27 Thread Edwin Vane
Author: revane Date: Tue Aug 27 10:44:26 2013 New Revision: 189358 URL: http://llvm.org/viewvc/llvm-project?rev=189358&view=rev Log: Adding a vector version of clang::tooling::shiftedCodePosition(). During the transition of clang::tooling::Replacements from std::set to std::vector, functions such

Re: [PATCH] cpp11-migrate: Add a transform that use pass-by-value in constructors

2013-08-27 Thread Edwin Vane
Comment at: docs/PassByValueTransform.rst:135 @@ +134,3 @@ + +When delayed template parsing is enabled, constructors with dependent types +aren't transformed. Delayed template parsing is enabled by default on Windows as Need a reword on "constructors with depend

Re: [PATCH] cpp11-migrate: Add a transform that use pass-by-value in constructors

2013-08-28 Thread Edwin Vane
Comment at: docs/PassByValueTransform.rst:136 @@ +135,3 @@ +When delayed template parsing is enabled, constructors part of templated +contexts aren't transformed. Delayed template parsing is enabled by default on +Windows as a Microsoft extension: `Clang Compiler User’s Manual -

Re: [PATCH] clang-replace: Afford applying replacements in memory

2013-08-28 Thread Edwin Vane
Closed by commit rL189493 (authored by @revane). CHANGED PRIOR TO COMMIT http://llvm-reviews.chandlerc.com/D1519?vs=3808&id=3869#toc http://llvm-reviews.chandlerc.com/D1519 COMMIT http://llvm-reviews.chandlerc.com/rL189493 ___ cfe-commits mailin

[clang-tools-extra] r189493 - clang-replace: Afford applying replacements in memory

2013-08-28 Thread Edwin Vane
Author: revane Date: Wed Aug 28 12:19:10 2013 New Revision: 189493 URL: http://llvm.org/viewvc/llvm-project?rev=189493&view=rev Log: clang-replace: Afford applying replacements in memory For users of libclangReplace, this patch affords the ability to apply replacements in memory instead of writin

Re: [PATCH] Transform all files in a compilation database if no source files are provided.

2013-08-29 Thread Edwin Vane
Sorry for taking so long to respond to this review. I've been heads down trying to prepare D1545 for review. I'm going to break this down into two cases: === 1. Explicitly provided files === If a user explicitly lists a source on the command-line it should be transformed. The only time

[PATCH] clang-replace: Re-org of file structure

2013-08-29 Thread Edwin Vane
Hi klimek, chandlerc, clang-replace is likely to move to clang proper one day soon. To facilitate that move, renaming files and directory structure layout to ease transition for users of clang-replace and libclangReplace. For now, functionality still exists in clang::replace namespace. Header gua

Re: [PATCH] clang-replace: Re-org of file structure

2013-08-29 Thread Edwin Vane
Replaced by D1548 which cleans up the patch and introduces a few more 'get ready for clang' changes. http://llvm-reviews.chandlerc.com/D1522 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Re: [PATCH] cpp11-migrate: Refactor for driver model of operation

2013-08-29 Thread Edwin Vane
Comment at: cpp11-migrate/Core/FileOverrides.cpp:192 @@ +191,3 @@ +std::string &S = FileStates[FileName]; +S.clear(); +llvm::raw_string_ostream StringStream(S); Tareq A. Siraj wrote: > Is it necessary to take a reference and clearing the string vs. wr

Re: [PATCH] clang-replace: Re-org of file structure

2013-08-30 Thread Edwin Vane
- Addressed reviewer comments Hi klimek, chandlerc, http://llvm-reviews.chandlerc.com/D1548 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1548?vs=3901&id=3924#toc Files: clang-replace/CMakeLists.txt clang-replace/Makefile clang-replace/include/clang-replace/Tooling/Apply

Re: [PATCH] clang-replace: Re-org of file structure

2013-08-30 Thread Edwin Vane
- Fixed header guards Hi klimek, chandlerc, http://llvm-reviews.chandlerc.com/D1548 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1548?vs=3924&id=3925#toc BRANCH reorg ARCANIST PROJECT clang-tools-extra Files: clang-replace/CMakeLists.txt clang-replace/Makefile cla

  1   2   3   4   5   6   7   >