Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RefactoringTool does not rewrite sources in another directory #22759

Open
maleadt opened this issue Jan 29, 2015 · 11 comments
Open

RefactoringTool does not rewrite sources in another directory #22759

maleadt opened this issue Jan 29, 2015 · 11 comments
Labels
bugzilla Issues migrated from bugzilla clang:as-a-library libclang and C++ API confirmed Verified by a second party

Comments

@maleadt
Copy link
Contributor

maleadt commented Jan 29, 2015

Bugzilla Link 22385
Version 3.6
OS Linux
CC @Sarcasm,@zmodem

Extended Description

If I use the RefactoringTool to match and rewrite files which reside in a subdirectory, matching works fine, but Rewriting fails because the sources are not found anymore (the directory component is stripped off).

For example, demonstrating using remove-cstr-calls from clang-tools-extra, if I have a file main.cpp in /tmp/test/src, and an accompanying compile_database.json (in the same directory) looking as follows:
[
{
"command": "g++ -c -o main.o main.cpp",
"directory": "/tmp/test/src",
"file": "/tmp/test/src/main.cpp"
}
]
(this compile database is generated using Bear)

If I now call the tooling binary from /tmp/test using remove-cstr-calls src src/main.cpp, I get:

fatal error: cannot open file 'main.cpp': No such file or directory
remove-cstr-calls: ../../tools/clang/include/clang/Rewrite/Core/RewriteRope.h:203: void clang::RewriteRope::erase(unsigned int, unsigned int): Assertion `Offset+NumBytes <= size() && "Invalid region to erase!"' failed.
#​0 0x7fc86c9a9c48 llvm::sys::PrintStackTrace(_IO_FILE*) ../../lib/Support/Unix/Signals.inc:422:15
#​1 0x7fc86c9ab24b SignalHandler(int) ../../lib/Support/Unix/Signals.inc:198:28
#​2 0x7fc86a7b4b20 __restore_rt (/usr/lib/libc.so.6+0x33b20)
#​3 0x7fc86a7b4a97 __GI_raise (/usr/lib/libc.so.6+0x33a97)
#​4 0x7fc86a7b5e6a __GI_abort (/usr/lib/libc.so.6+0x34e6a)
#​5 0x7fc86a7ad8bd __assert_fail_base (/usr/lib/libc.so.6+0x2c8bd)
#​6 0x7fc86a7ad972 (/usr/lib/libc.so.6+0x2c972)
#​7 0x7fc86798f7ce (bin/../lib/../lib/libclangRewrite.so+0x87ce)
#​8 0x7fc867990788 clang::Rewriter::ReplaceText(clang::SourceLocation, unsigned int, llvm::StringRef) ../../tools/clang/lib/Rewrite/Rewriter.cpp:311:3
#​9 0x7fc86b35105d clang::tooling::Replacement::apply(clang::Rewriter&) const ../../tools/clang/lib/Tooling/Core/Replacement.cpp:73:28
#​10 0x7fc86b351a3e clang::tooling::applyAllReplacements(std::set<clang::tooling::Replacement, std::lessclang::tooling::Replacement, std::allocatorclang::tooling::Replacement > const&, clang::Rewriter&) ../../tools/clang/lib/Tooling/Core/Replacement.cpp:234:16
#​11 0x7fc86b567898 clang::tooling::RefactoringTool::runAndSave(clang::tooling::FrontendActionFactory*) ../../tools/clang/lib/Tooling/Refactoring.cpp:48:8
#​12 0x404eed main ../../tools/clang/tools/extra/remove-cstr-calls/RemoveCStrCalls.cpp:236:10
#​13 0x7fc86a7a1040 __libc_start_main (/usr/lib/libc.so.6+0x20040)
#​14 0x402f78 _start (bin/remove-cstr-calls+0x402f78)
Aborted (core dumped)

If I call the binary from /tmp/test/src (the directory where main.cpp resides) using remove-cstr-calls . main.cpp, everything works fine and the file is properly rewritten.

I'm using LLVM/Clang from the 3.6 branch, at revision 227398.

@maleadt
Copy link
Contributor Author

maleadt commented Feb 1, 2015

I've tested this bug with remove-cstr-calls compiled against LLVM/Clang 3.5.1, and everything works as expected (refactoring files in a subdirectory, that is). So this seems like a regression from 3.5 to 3.6.

Some more details, the file I tested to rewrite:

----main.cpp--------------------
#include

int main() {
std::string Hello = "World";
std::out << std::string(Hello.c_str()) << std::endl;

return 1;

}

I checked out RemoveCstrCalls.cpp from release_35, compiled it against my system 3.5.1 LLVM installation, and called the binary as follows:

in /tmp/test:
$ llvm-3.5/remove-cstr-calls src src/main.cpp

This works without error, while using remove-cstr-calls from 3.6 fails.

@maleadt
Copy link
Contributor Author

maleadt commented Feb 3, 2015

I've bisected this to r221600.

@zmodem
Copy link
Collaborator

zmodem commented Feb 4, 2015

I've bisected this to r221600.

Alex, can you take a look?

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 4, 2015

The problem is that the paths in replacements turn out to be relative. That seems strange to me, as Replacement::setFromSourceLocation which seems to be used there, makes paths absolute.

I'll take a look at this tomorrow.

@zmodem
Copy link
Collaborator

zmodem commented Feb 6, 2015

Has there been any progress on this one?

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 9, 2015

Not yet, unfortunately. Trying to switch to this.

@zmodem
Copy link
Collaborator

zmodem commented Feb 12, 2015

Not yet, unfortunately. Trying to switch to this.

Any update?

I'm not sure if this is worth blocking the release on, but it sounds like it would be nice to get fixed.

@maleadt
Copy link
Contributor Author

maleadt commented Feb 12, 2015

It seems like Replacement::setFromSourceLocation fails to make the path absolute (fs::make_absolute fails) because the FileEntry it gets from the SourceManager does not contain a full path. In case of the test set-up I used above, the FileEntry contains file="main.cpp" dir="."
I tried to look further but couldn't easily find where the SourceManager is populated.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2015

Hans, I think, this shouldn't block the release, as there's an easy workaround (run from the directory where the compilation database points to).

But yeah, it would be nice to fix, and I'm looking into this. BTW, clang-check fails this way when trying to print an error message as well.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 12, 2015

I meant clang-tidy crashes when outputting an error message in this case. And clang-check works fine.

@zmodem
Copy link
Collaborator

zmodem commented Feb 12, 2015

Hans, I think, this shouldn't block the release, as there's an easy
workaround (run from the directory where the compilation database points to).

Sounds reasonable.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:as-a-library libclang and C++ API confirmed Verified by a second party
Projects
None yet
Development

No branches or pull requests

3 participants