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

Fix make check with newer versions of clang. #93

Closed
wants to merge 3 commits into from

Conversation

elijahr
Copy link

@elijahr elijahr commented Jan 26, 2020

make check with Apple clang version 11.0.0 (clang-1100.0.33.16) fails with the following:

$ make check
Making check in src
Making check in .
g++ -DHAVE_CONFIG_H -I. -I..   -g -O0 -I.. -std=c++11 -Qunused-arguments -O0 -g -Wall -Werror -DDEBUG -MT cpp_test-cpp_test.o -MD -MP -MF .deps/cpp_test-cpp_test.Tpo -c -o cpp_test-cpp_test.o `test -f 'cpp_test.cpp' || echo './'`cpp_test.cpp
cpp_test.cpp:126:34: error: lambda capture 'j' is not used [-Werror,-Wunused-lambda-capture]
    s.add_method("test12", "i", [j](const lo::Message m)
                                 ^
1 error generated.
make[2]: *** [cpp_test-cpp_test.o] Error 1
make[1]: *** [check-recursive] Error 1
make: *** [check-recursive] Error 1

I tried changing this in src/Makefile.am:

cpp_test_CXXFLAGS = -I$(top_srcdir)

to

cpp_test_CXXFLAGS = -I$(top_srcdir) -Wunused-lambda-capture

But -Werror from CXXFLAGS overrides cpp_test_CXXFLAGS in the generated Makefile because of the order of arguments:

cpp_test-cpp_test.o: cpp_test.cpp
	$(AM_V_CXX)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(cpp_test_CXXFLAGS) $(CXXFLAGS) -MT cpp_test-cpp_test.o -MD -MP -MF $(DEPDIR)/cpp_test-cpp_test.Tpo -c -o cpp_test-cpp_test.o `test -f 'cpp_test.cpp' || echo '$(srcdir)/'`cpp_test.cpp
	$(AM_V_at)$(am__mv) $(DEPDIR)/cpp_test-cpp_test.Tpo $(DEPDIR)/cpp_test-cpp_test.Po

So, I think implementing this as a clang-specific pragma makes sense. If there is a way to do this in Makefile.am rather than in the source file I would love to know how. This ended up being a much simpler fix than I initially thought.

I've also updated the travis config to run tests with this version of clang on OSX.

Build: https://travis-ci.org/elijahr/liblo/builds/642682851

`make check` with Apple clang version 11.0.0 (clang-1100.0.33.16) fails with the following:

```
$ make check
Making check in src
Making check in .
g++ -DHAVE_CONFIG_H -I. -I..   -g -O0 -I.. -std=c++11 -Qunused-arguments -O0 -g -Wall -Werror -DDEBUG -MT cpp_test-cpp_test.o -MD -MP -MF .deps/cpp_test-cpp_test.Tpo -c -o cpp_test-cpp_test.o `test -f 'cpp_test.cpp' || echo './'`cpp_test.cpp
cpp_test.cpp:126:34: error: lambda capture 'j' is not used [-Werror,-Wunused-lambda-capture]
    s.add_method("test12", "i", [j](const lo::Message m)
                                 ^
1 error generated.
make[2]: *** [cpp_test-cpp_test.o] Error 1
make[1]: *** [check-recursive] Error 1
make: *** [check-recursive] Error 1
```

I tried changing this in `src/Makefile.am`:

```
cpp_test_CXXFLAGS = -I$(top_srcdir)
```

to

```
cpp_test_CXXFLAGS = -I$(top_srcdir) -Wunused-lambda-capture
```

But `-Werror` from `CXXFLAGS` overrides `cpp_test_CXXFLAGS` in the generated Makefile because of the generated order of arguments:

```
cpp_test-cpp_test.o: cpp_test.cpp
	$(AM_V_CXX)$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(cpp_test_CXXFLAGS) $(CXXFLAGS) -MT cpp_test-cpp_test.o -MD -MP -MF $(DEPDIR)/cpp_test-cpp_test.Tpo -c -o cpp_test-cpp_test.o `test -f 'cpp_test.cpp' || echo '$(srcdir)/'`cpp_test.cpp
	$(AM_V_at)$(am__mv) $(DEPDIR)/cpp_test-cpp_test.Tpo $(DEPDIR)/cpp_test-cpp_test.Po
```

So, I think implementing this as a clang-specific pragma makes sense. If there is a way to do this in Makefile.am rather than in the source file I would love to know how.
@elijahr elijahr mentioned this pull request Jan 26, 2020
@radarsat1
Copy link
Owner

Hi, isn't it a lot easier to just fix the error and either add a reference to j or remove it? I'll do this.

@radarsat1
Copy link
Owner

Fixed in 9ab30f7

@radarsat1 radarsat1 closed this Jan 29, 2020
@elijahr elijahr deleted the elijahr/osx-cpp-test-fix branch May 14, 2020 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants