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

[Xcode] Compile failure with ARC enabled in program with an Objective-C++ test runner. #1661

Open
phniix opened this issue Jun 18, 2019 · 7 comments

Comments

@phniix
Copy link

phniix commented Jun 18, 2019

Describe the bug

Programs containing Catch2's single_include header fail to compile in Xcode projects that:

  • have the target's Build Setting option, Objective-C Automatic Reference Counting set to Yes; and,
  • define CATCH_CONFIG_MAIN in an Objective-C++ .mm file.

The following compile time errors are experienced:

In file included from /Users/phx/skrach/catch2_arc_compile_bug/catch2arc_compilation_error/catch2arc_compilation_error/main.mm:3:
include/catch2/catch.hpp:4714:58: error: non-virtual member function marked 'override' hides virtual member function
                bool match( NSString* const& str ) const override {
                                                         ^
include/catch2/catch.hpp:3180:26: note: hidden overloaded virtual function 'Catch::Matchers::Impl::MatcherMethod<NSString *>::match' declared here: type mismatch at 1st parameter ('NSString *__strong const &' vs 'NSString *const __autoreleasing &')
            virtual bool match( ObjectT const& arg ) const = 0;
                         ^
include/catch2/catch.hpp:4724:58: error: only virtual member functions can be marked 'override'
                bool match( NSString* const& str ) const override {
                                                         ^
include/catch2/catch.hpp:4737:58: error: only virtual member functions can be marked 'override'
                bool match( NSString* const& str ) const override {
                                                         ^
include/catch2/catch.hpp:4750:58: error: only virtual member functions can be marked 'override'
                bool match( NSString* const& str ) const override {
                                                         ^
include/catch2/catch.hpp:4762:58: error: only virtual member functions can be marked 'override'
                bool match( NSString* const& str ) const override {
                                                         ^
include/catch2/catch.hpp:4776:13: error: return type 'Impl::NSStringMatchers::Equals' is an abstract class
            Equals( NSString* substr ){ return Impl::NSStringMatchers::Equals( substr ); }
            ^
include/catch2/catch.hpp:3180:26: note: unimplemented pure virtual method 'match' in 'Equals'
            virtual bool match( ObjectT const& arg ) const = 0;
                         ^
include/catch2/catch.hpp:4776:48: error: allocating an object of abstract class type 'Impl::NSStringMatchers::Equals'
            Equals( NSString* substr ){ return Impl::NSStringMatchers::Equals( substr ); }
                                               ^
include/catch2/catch.hpp:4779:13: error: return type 'Impl::NSStringMatchers::Contains' is an abstract class
            Contains( NSString* substr ){ return Impl::NSStringMatchers::Contains( substr ); }
            ^
include/catch2/catch.hpp:3180:26: note: unimplemented pure virtual method 'match' in 'Contains'
            virtual bool match( ObjectT const& arg ) const = 0;
                         ^
include/catch2/catch.hpp:4779:50: error: allocating an object of abstract class type 'Impl::NSStringMatchers::Contains'
            Contains( NSString* substr ){ return Impl::NSStringMatchers::Contains( substr ); }
                                                 ^
include/catch2/catch.hpp:4782:13: error: return type 'Impl::NSStringMatchers::StartsWith' is an abstract class
            StartsWith( NSString* substr ){ return Impl::NSStringMatchers::StartsWith( substr ); }
            ^
include/catch2/catch.hpp:3180:26: note: unimplemented pure virtual method 'match' in 'StartsWith'
            virtual bool match( ObjectT const& arg ) const = 0;
                         ^
include/catch2/catch.hpp:4782:52: error: allocating an object of abstract class type 'Impl::NSStringMatchers::StartsWith'
            StartsWith( NSString* substr ){ return Impl::NSStringMatchers::StartsWith( substr ); }
                                                   ^
include/catch2/catch.hpp:4785:13: error: return type 'Impl::NSStringMatchers::EndsWith' is an abstract class
            EndsWith( NSString* substr ){ return Impl::NSStringMatchers::EndsWith( substr ); }
            ^
include/catch2/catch.hpp:3180:26: note: unimplemented pure virtual method 'match' in 'EndsWith'
            virtual bool match( ObjectT const& arg ) const = 0;
                         ^
include/catch2/catch.hpp:4785:50: error: allocating an object of abstract class type 'Impl::NSStringMatchers::EndsWith'
            EndsWith( NSString* substr ){ return Impl::NSStringMatchers::EndsWith( substr ); }
                                                 ^
13 errors generated.

These errors are resolved when the Build Setting, Objective-C Automatic Reference Counting is set to NO.

Expected behavior
To be able to compile the program.

Reproduction steps
Steps to reproduce the bug.

For iOS

  1. Create a new project from File->New->Project. Make sure language selected is Objective-C.
  2. Select 'Single View App' from the iOS templates.
  3. Go to Build Settings:
    • Confirm Objective-C Automatic Reference Counting is set to Yes.
    • Add your catch2 single include header path to Header Search Paths.
  4. Rename main.m to main.mm
  5. Delete AppDelegate.* and ViewController.* files.
  6. Replace content of main.mm with:
#define CATCH_CONFIG_MAIN
#include <catch2/catch.hpp>
  1. Compile the program.

For macOS
Follow steps 1-7 from For iOS, substituting 2 with:
2. Select 'Cocoa App' from the macOS templates.

Platform information:

  • OS: macOS Mojave 10.14.5
  • Compiler+version: Xcode 10.2.1 (10E1001), Xcode 11.0 beta (11M336w)
  • Catch version: v2.7.2 -- v2.9.1

Additional context

I have attached a sample project that exhibits this behavior. The sample can be found in the attachment: catch2arc_compilation_error.zip.

This sample is packaged with Catch version v2.9.1.

After uncompressing the file, open the pre-built Xcode project: catch2arc_compilation_error.xcodeproj
In the project, there are two schemes:

  • catch2arc_compilation_error
  • catch2arc_compilation_success

The targets for each scheme are:

  • catch2arc_compilation_error: iOS
  • catch2arc_compilation_success: macOS

The Build Setting Objective-C Automatic Reference Counting for targets is set to:

  • catch2arc_compilation_error: Yes
  • catch2arc_compilation_success: No

Compile the scheme catch2arc_compilation_error to see the errors.

@phniix phniix changed the title [Xcode] Compile failure with ARC enabled in with an Objective-C++ test runner. [Xcode] Compile failure with ARC enabled in program with an Objective-C++ test runner. Jun 18, 2019
@bdb
Copy link

bdb commented Aug 6, 2019

Same bug with Xcode 10.2.1 and Catch 2.9.1

Adding 'virtual' to the NSString* overrides seems to be the fix (and it should be present no matter what). However I tried this and am now getting the following:

catch.hpp:4714:30: error: 'match' marked 'override' but
      does not override any member functions
                virtual bool match( NSString* const& str ) const override {

catch.hpp:4776:13: error: return type
      'Impl::NSStringMatchers::Equals' is an abstract class
            Equals( NSString* substr ){ return Impl::NSStringMatchers::Equals( substr ); }

This repeats for each override method. So it appears the override signature is different from the base signature. This is probably due to the const qualifier which has caused problems before with Catch and Objc. Variants of this issue keep cropping up. See #1571 and #278.

@bdb
Copy link

bdb commented Aug 6, 2019

In Catch 2.4, MatcherBase had a template override for pointers. This override contained a match signature without 'const'. I suspect whenever this was removed was when the NSString* code broke:

template<typename PtrT>
        struct MatcherMethod<PtrT*> {
            virtual bool match( PtrT* arg ) const = 0;
        };

@bdb
Copy link

bdb commented Aug 6, 2019

Here's a patch against 2.9.1 single-header that seems to fix the problem. Our internal tests now build and pass. I don't know Catch2 well enough to get a test case, but I'll see if I can figure it out. It would be nice to have this bug stop cropping up by getting some basic ObjC++ tests.

--- a/prosoft/core/tests/catch.hpp
+++ b/prosoft/core/tests/catch.hpp
@@ -3180,6 +3180,14 @@ namespace Matchers {
             virtual bool match( ObjectT const& arg ) const = 0;
         };
 
+#if __OBJC__
+        template<> // Hack to fix Catch GH issue #1661. Could use id for generic Object support.
+        // use of const for Object pointers is very uncommon and under ARC it causes some kind of signature mismatch that breaks compilation
+        struct MatcherMethod<NSString*> {
+            virtual bool match( NSString* arg ) const = 0;
+        };
+#endif
+
 #ifdef __clang__
 #    pragma clang diagnostic pop
 #endif
@@ -4711,7 +4719,7 @@ namespace Catch {
                     arcSafeRelease( m_substr );
                 }
 
-                bool match( NSString* const& str ) const override {
+                bool match( NSString* str ) const override {
                     return false;
                 }
 
@@ -4721,7 +4729,7 @@ namespace Catch {
             struct Equals : StringHolder {
                 Equals( NSString* substr ) : StringHolder( substr ){}
 
-                bool match( NSString* const& str ) const override {
+                bool match( NSString* str ) const override {
                     return  (str != nil || m_substr == nil ) &&
                             [str isEqualToString:m_substr];
                 }
@@ -4734,7 +4742,7 @@ namespace Catch {
             struct Contains : StringHolder {
                 Contains( NSString* substr ) : StringHolder( substr ){}
 
-                bool match( NSString* const& str ) const override {
+                bool match( NSString* str ) const override {
                     return  (str != nil || m_substr == nil ) &&
                             [str rangeOfString:m_substr].location != NSNotFound;
                 }
@@ -4747,7 +4755,7 @@ namespace Catch {
             struct StartsWith : StringHolder {
                 StartsWith( NSString* substr ) : StringHolder( substr ){}
 
-                bool match( NSString* const& str ) const override {
+                bool match( NSString* str ) const override {
                     return  (str != nil || m_substr == nil ) &&
                             [str rangeOfString:m_substr].location == 0;
                 }
@@ -4759,7 +4767,7 @@ namespace Catch {
             struct EndsWith : StringHolder {
                 EndsWith( NSString* substr ) : StringHolder( substr ){}
 
-                bool match( NSString* const& str ) const override {
+                bool match( NSString* str ) const override {
                     return  (str != nil || m_substr == nil ) &&
                             [str rangeOfString:m_substr].location == [str length] - [m_substr length];
                 }

@horenmar
Copy link
Member

horenmar commented Aug 7, 2019

My standing thoughts on Objective-C++ is that I don't use it and I am not planning to learn it either, but if someone is willing to set up CI, I am willing to avoid breaking it.

horenmar added a commit that referenced this issue Aug 7, 2019
Thanks to bdb for the patch, related to #1661
@horenmar
Copy link
Member

horenmar commented Aug 7, 2019

@bdb Should be fixed in master, can you test it? (You will need to regenerate the single header with scripts/generateSingleHeader.py.)

@bdb
Copy link

bdb commented Aug 7, 2019

Will do @horenmar. I'll try to find some time to work on a basic test case too.

@phniix
Copy link
Author

phniix commented Aug 10, 2019

Hi Brian @bdb ,
Thank you so much for picking this up and running with it. My Objective-C++/C kungfu was not up to the task of handling this. I have learned a bit with your fix in place. I was then armed to look into this a bit more, and found this: Does the concept of “const reference” in C++ exists in Objective-C ?

Today, I have learned kung-fu. Thank you.

If only I had the where-with-all to ask that question or come to such reasoning when tracing this elusive and rather horrendous compiler bug. You are a champion for hunting this down and going back to Catch 2.4.. Good onya mate!

One thing about one of your prior comments about the missing virtual:

Adding 'virtual' to the NSString* overrides seems to be the fix (and it should be present no matter what).

In C++11 onwards, the virtual specifier is optional (cppreference.com link). I think the 'should be present no matter what' is definitely a style preference, and I think it is great that you mention it. I like the approach of not specifying the virtual explicitly for derived class implementations, and I think its mainly because it makes me feel like I'm writing more modern code.

I saw that this fix is in Catch2 v2.9.2, so I will pull that down and run our test suite against it. This is awesome stuff!

Hi Martin @horenmar ,
Thank you for taking on board the Objective-C++ perspective. I was really overjoyed when we could use our existing Catch2 tests and plug them into Objective-C++ runners with hardly any effort. This has reduced our maintenance and test burden a fair bit. It was a bit of a bummer that we had to fork and patch to remove the offending code for last number of versions of Catch2. I am super happy that it appears we can once again come into alignment with Catch2/master.

Now, I'm not sure if a basic smoke screen test like the one I provided as a test case for this issue would serve as a nice basis for a simple, "can we still compile test case". Given Brian's digging back to v2.4, it might a nice addition to a regression test suite.

Thanking you both for nailing this issue, and I can't wait to try out the new release!
All the best,
pheonix

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

No branches or pull requests

3 participants