An interface as is presented with git add -p is just an idea for future work
at this point. Right now, cpp11-migrate is going to apply transforms wholesale
to files with no user input other than what is given on the command line. As
mentioned in the design doc, there are plans to introduce more ways for user
input especially as the transforms get more complicated. This is basically the
very first commit for this tool. We have to start somewhere after all.
================
Comment at: cpp11-migrate/Cpp11Migrate.cpp:39-42
@@ +38,6 @@
+// Helper RAII class for created Transforms.
+class TransformVec {
+public:
+ typedef std::vector<Transform*>::const_iterator const_iterator;
+
+public:
----------------
Sean Silva wrote:
> Can you just use a regular vector of smart pointers?
I can do that. Does LLVM have a utility smart pointer that's recommended?
================
Comment at: cpp11-migrate/Cpp11Migrate.cpp:82-83
@@ -40,1 +81,4 @@
+
+#define AddTransformOpt(List, Name, T, Desc) \
+ List.getParser().addLiteralOption(Name, &ConstructTransform<T>, Desc);
----------------
Sean Silva wrote:
> Why is this a macro?
It mirrors the EnumValN macro from the command line library. The point is to
make adding options for transforms nicer.
================
Comment at: cpp11-migrate/Cpp11Migrate.cpp:123
@@ +122,3 @@
+ // Final Syntax check.
+ if (!TransformList.empty()) {
+ ClangTool EndSyntaxTool(OptionsParser.getCompilations(),
----------------
Sean Silva wrote:
> Not specifying any transforms should probably just be an error during option
> parsing.
Just an arbitrary decision I made. I can make it at error if that's preferred.
================
Comment at: cpp11-migrate/Cpp11Migrate.cpp:124-129
@@ +123,8 @@
+ if (!TransformList.empty()) {
+ ClangTool EndSyntaxTool(OptionsParser.getCompilations(),
+ OptionsParser.getSourcePathList());
+ if (EndSyntaxTool.run(
+ newFrontendActionFactory<clang::SyntaxOnlyAction>()) != 0) {
+ return 1;
+ }
+ }
----------------
Sean Silva wrote:
> What is the purpose of doing this if it doesn't roll back the changes?
>
> If the tool "fails to transform my files" (`return 1`), then I expect it to
> not modify my files (or at least leave them in some well-defined state; or at
> the absolute minimum give me a useful diagnostic explaining the situation).
> The same could be said for the `return 1` in the transform-apply loop.
Reverting the state of files is something that's not fully implemented due to
the simple interface provided by rewriting. The point of this commit is simply
to port the loop convert tool into the cpp11-migrate tool to get things moving.
Some work is going to need to be done with clang itself to get things to work
properly according to the design.
================
Comment at: cpp11-migrate/Transform.h:58-59
@@ +57,4 @@
+private:
+ bool ChangesMade;
+ bool ChangesNotMade;
+};
----------------
Sean Silva wrote:
> Why are there two flags here? When would one not be the opposite of the
> other? Maybe a better name is necessary?
This is a port of loop-convert functionality. Loop-convert kept track of
changes made, changes it might have made if they did not overlap with other
changes (deferred it called them), and changes that it would have made if the
user's tolerance for risk was high enough (rejected). Since I'm not sure what
cpp11-migrate is going to do with regards to tracking that info yet I just
hooked up the loop-convert functionality to these flags. 'changes made'
indicates changes were actually made. 'changes-not-made' means there were
changes not made (deferred or rejected by original nomenclature).
I didn't spend a lot of time thinking about nice names considering the
uncertain nature of this functionality.
http://llvm-reviews.chandlerc.com/D251
BRANCH
loop-convert
ARCANIST PROJECT
clang-tools-extra
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
