This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Don't rename the namespace.
ClosedPublic

Authored by hokein on Jun 25 2019, 3:41 AM.

Details

Summary

Also fix a small bug -- the extra argument "-xc++" doesn't overwrite the
language if the argument is present after the file name in the compiler
command.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein created this revision.Jun 25 2019, 3:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2019, 3:41 AM
sammccall accepted this revision.Jun 25 2019, 3:52 AM
sammccall added inline comments.
clang-tools-extra/clangd/refactor/Rename.cpp
167 ↗(On Diff #206402)

can't rename symbols of this kind?

"supported" lacks context I think.

clang-tools-extra/clangd/unittests/RenameTests.cpp
137 ↗(On Diff #206402)

(why this change?)

This revision is now accepted and ready to land.Jun 25 2019, 3:52 AM
hokein updated this revision to Diff 206412.Jun 25 2019, 4:27 AM
hokein marked 3 inline comments as done.

Address review comments.

clang-tools-extra/clangd/unittests/RenameTests.cpp
137 ↗(On Diff #206402)

for the cases here, we want the main file treat as a header file, using -xc++ here would make clang treat it as a .cc file.

sammccall added inline comments.Jun 25 2019, 6:25 AM
clang-tools-extra/clangd/unittests/RenameTests.cpp
137 ↗(On Diff #206402)

It sounds like this is unrelated to the current change, and is designed to address tests that were passing by mistake (rename was failing because the file was not a header, not for the desired reason.

Can we split up the fix into another patch, and verify it by asserting on the error message?

hokein updated this revision to Diff 206437.Jun 25 2019, 7:03 AM
hokein marked an inline comment as done.

Verify the error message in the test.

hokein added inline comments.Jun 25 2019, 7:03 AM
clang-tools-extra/clangd/unittests/RenameTests.cpp
137 ↗(On Diff #206402)

Done in this patch, adding the error message when doing the verification.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2019, 1:11 AM