This is an archive of the discontinued LLVM Phabricator instance.

Rename three cxx files in unittests to cpp.
ClosedPublic

Authored by thakis on May 14 2018, 1:00 PM.

Details

Reviewers
rnk
philip.pfaffe
Summary

LLVM uses cpp as its C++ file extension, these are the only three cxx file in the monorepo. These files apparently were called to escape a CMake check -- use the LLVM_OPTIONAL_SOURCES mechanism that's meant as an escape for this case instead.

No intended behavior change.

Diff Detail

Event Timeline

thakis created this revision.May 14 2018, 1:00 PM

Something is off with the diff. E.g. the file TestPlugin.cpp doesn't exist, yet the diff contains modifications. Did you miss a local commit?

unittests/Passes/CMakeLists.txt
1

It's not a hack, but the purpose of that setting. So just document the intent directly.

Something is off with the diff. E.g. the file TestPlugin.cpp doesn't exist, yet the diff contains modifications. Did you miss a local commit?

That's how svn diff looks after svn mv. Maybe you use git (which doesn't have a native "move file" operation) and aren't used to seeing this type of diff?

unittests/Passes/CMakeLists.txt
1

Fair enough. This wording is used in a few places (http://llvm-cs.pcc.me.uk/?q=Hack+to+bypass+LLVM); will change those too once this lands.

That's how svn diff looks after svn mv. Maybe you use git (which doesn't have a native "move file" operation) and aren't used to seeing this type of diff?

I'm not sure what caused it, but the patch is not applicable.

After some digging, I believe this is caused by svn diff comparing the new file against the old one, which is of course unsuitable to create patches from. So you likely should create the diff with svn diff --show-copies-as-adds or svn diff --patch-compatible.

That's how svn diff looks after svn mv. Maybe you use git (which doesn't have a native "move file" operation) and aren't used to seeing this type of diff?

I'm not sure what caused it, but the patch is not applicable.

I used the command that https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface tells me to use. As far as I know, svn diff doesn't produce diffs that apply for svn mv changes. It'll commit ok.

philip.pfaffe accepted this revision.May 15 2018, 7:51 AM

Since the diff being broken is mostly a workflow issue, this patch LGTM.

This revision is now accepted and ready to land.May 15 2018, 7:51 AM

r332368, thanks!

thakis closed this revision.May 15 2018, 9:34 AM
unittests/Support/DynamicLibrary/ExportedFuncs.cpp