[cfe-dev] [PATCH] Refactoring: fix making absolute FilePath in Replacement

[Manuel Klimek]

On Tue Sep 02 2014 at 2:38:59 PM Steffen Prohaska <prohaska at zib.de> wrote:

>
> On Sep 2, 2014, at 1:41 AM, Justin Bogner <mail at justinbogner.com> wrote:
>
> > Justin Bogner <mail at justinbogner.com> writes:
> >> Steffen Prohaska <prohaska at zib.de> writes:
> >>> An std::error_code of 0 indicates success.  The condition must be
> reversed.
> >>>
> >>> See diff below for http://llvm.org/svn/llvm-project/cfe/trunk
> >>> ---
> >>>
> >>> Hello,
> >>> I'm unsure whether this is the right way to submit a patch.  Please
> let me
> >>> know if I should prepare it in a different way.
> >>
> >> Generally, we prefer patches to be sent to cfe-commits@, rather than
> >> cfe-dev@, for future reference.
> >>
> >> Additionally, we usually like to check in a test with bug fixes. This is
> >> pretty obvious by inspection, but if you have a case where it fails and
> >> can easily be reduced, submitting that as well would be appreciated.
> >
> > On further inspection, fixing this opens a whole can of worms. It looks
> > like we've been relying on it being broken for quite a while. I've sent
> > an alternative patch to cfe-commits for review:
> >
> >    http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-
> Mon-20140901/113949.html
>
> Hmm...  Your patch looks sane, but it doesn't solve my original problem.
>
> Maybe the following is relevant.  I'm unsure, because I'm rather
> unfamiliar with the clang codebase.
>
> I've been working on a custom refactoring tool.  I tried to save the
> replacements and then apply them with clang-apply-replacements.  I
> wanted to collect replacements for several packages across a large
> project.  The problem can be illustrated with clang-modernize.
>
> I created a compile_commands.json for a custom build system that uses
> recursive make. Commands are executed in the subdirectory of the source
> file:
>
>     [
>         {
>             "directory": "/rootpath/src/package",
>             "command": "clang++ ... file.cpp",
>             "file": "file.cpp"
>         },
>         ...
>     ]
>
> I'd like to create serialized replacements similar to:
>
>     clang-modernize --add-override --serialize-replacements \
>         --serialize-dir=$(pwd)/replacements src/package/file.cpp
>
> and then apply them with:
>
>     clang-apply-replacements -format ./replacements
>
> The setup seemed natural, and I expected it to work this way.  But it
> fails, because the serialized replacements contain paths that are
> relative to the directories in compile_commands.json (like
> "FilePath: file.cpp").
>
> With my fix that teaches Replacement to store absolute paths, it works
> as expected, because the Replacements and therefore the YAML files then
> contain absolute paths.
>

The problem is that this then leaves you with replacements that are not
relocatable; for example, if you run the refactoring tool distributed over
multiple machines, where each machine may have a random path component
(machine dependent), you'll end up with replacements you cannot apply at
all.

Which format are you outputting replacements in? Can you just call
make_absolute on the replacement's path when you output them?


>
> Best,
>   Steffen
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140902/91a37f41/attachment.html>