Page MenuHomePhabricator

Remove test for g++ 4.6
ClosedPublic

Authored by emaste on May 29 2015, 7:40 AM.

Details

Summary

Skip the g++ 4.6 test if we're not going to build any C++ source. If a test has C++ source files we automatically determine which C++ compiler to use based on $CC (for example, clang++ if CC=clang). However, this is not done for tests without C++ source and CXX will be GNU make's default of g++. This produces suprious "g++: not found" errors in testrun output on systems without a gcc/g++.

Diff Detail

Repository
rL LLVM

Event Timeline

emaste updated this revision to Diff 26780.May 29 2015, 7:40 AM
emaste retitled this revision from to Improve test for g++ 4.6.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: labath, chaoren.
emaste added a subscriber: Unknown Object (MLST).
chaoren edited edge metadata.May 29 2015, 7:46 AM

$(shell $(CXX) g++ -dumpversion | cut -b 1-3)

Do you mean

$(shell $(CXX) -dumpversion | cut -b 1-3)

?

emaste updated this revision to Diff 26781.May 29 2015, 7:54 AM
emaste edited edge metadata.

Remove c++ accidentally left behind,

chaoren added inline comments.May 29 2015, 8:58 AM
test/make/Makefile.rules
346 ↗(On Diff #26781)

I think we should use findstring instead of filter here, since filter can only match g++ exactly and ignore things like special-custom-g++ and /path/to/g++. Also, it's a little weird to have 2 ways of comparing empty strings, I think we should standardize on ifneq "string 1" "string 2".

But that would also match clang++. Maybe we should check for gcc in
$(CC)?

Or, perhaps we can just remove this check / c++0x workaround altogether now? Since LLVM/Clang/LLDB requires a C++11 compiler to build, we know the user has one they can use.

I support that idea, but we should probably consult Greg as well.

I support that idea, but we should probably consult Greg as well.

I've added Greg, but I don't think it will be a concern. This bit originated with the Intel developers when they were working on LLDB/Linux. At this point I think your team would have the best handle on whether GCC 4.6 is a relevant compiler in the Linux ecosystem. On both OS X and FreeBSD Clang has been the primary compiler for some time.

emaste updated this revision to Diff 26792.May 29 2015, 10:17 AM
emaste retitled this revision from Improve test for g++ 4.6 to Remove test for g++ 4.6.

Just remove the c++0x workaround for GCC 4.6. It's no longer of interest, and we know the user has a C++11 compiler (in order to build LLDB).

chaoren accepted this revision.May 29 2015, 10:22 AM
chaoren edited edge metadata.
This revision is now accepted and ready to land.May 29 2015, 10:22 AM

Perhaps a more general solution of checking whether c++11 is supported? Like try invoking $(CC) with -std=c++11, if it fails we use -std=c__0x instead. We don't have to tie it to a specific gcc version.

On 29 May 2015 at 13:54, Vince Harron <vince@nethacker.com> wrote:

Debugging apps built with gcc 4.6 might be important to someone. No reason
to pull it out if it's not hurting.

Perhaps we can just expect the user to set CXXFLAGS=-std=c++0x?

Anyway, I'm happy enough to commit the previous version of the patch instead (adding ifneq "$(strip $(CXX_SOURCES) $(OBJCXX_SOURCES))" "")

Okay, LGTM on the previous diff (26781).

This revision was automatically updated to reflect the committed changes.

Okay, LGTM on the previous diff (26781).

Thanks, I committed that version (with a change to the comparison style as you suggested - I agree it's odd to have two different styles back to back).