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

Add basic C++ support #43

Merged
merged 3 commits into from
Sep 20, 2020
Merged

Add basic C++ support #43

merged 3 commits into from
Sep 20, 2020

Conversation

aceckel
Copy link
Contributor

@aceckel aceckel commented Aug 20, 2020

This commit adds C++ support with the following modifications:

  • Remove the use of C-only features including designated initializers
    and non-strict prototypes.
  • Add spaces between literals and string macros to silence the C++-only
    -Wliteral-suffix warning.
  • Conditionally use template specialization instead of tentative
    definitions to implement optional setup/teardown functions.

Note that no C++-specific features have been added. The goal of this
commit is to simply enable compiling in C++ mode.

This commit adds C++ support with the following modifications:
- Remove the use of C-only features including designated initializers
  and non-strict prototypes.
- Add spaces between literals and string macros to silence the C++-only
  -Wliteral-suffix warning.
- Conditionally use template specialization instead of tentative
  definitions to implement optional setup/teardown functions.

Note that no C++-specific features have been added.  The goal of this
commit is to simply enable compiling in C++ mode.
@aceckel aceckel mentioned this pull request Aug 20, 2020

#ifdef __cplusplus

#define CTEST_SETUP(sname) \
Copy link
Contributor Author

@aceckel aceckel Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handling of the optional setup/teardown functions is the only place where the actual logic must differ between the C and C++ compilation. The problem is that C++ doesn't support tentative definitions the way that C does, which we currently rely on. Having 2 separate approaches to solve the same problem isn't ideal, so it would be nice if we could come up with an approach that both the C and C++ could share.

One potential option is to use a hashtable to store the setup and teardown function pointers, keyed by suite name. Then we could populate this hashtable using __attribute__ ((constructor)) functions that we would install as part of the CTEST_SETUP()/CTEST_TEARDOWN macros. Here is an example implementation (leaving out the hashtable implementation for brevity):

typedef void (*ctest_fixture_func)(void*);

struct ctest_suite {
    const char* name;
    ctest_fixture_func setup;
    ctest_fixture_func teardown;
    struct ctest_slist_head list; // linux kernel style intrusive list head
};

#define CTEST_IMPL_FIXTURE(sname, member, FNAME, CNAME) \
    __attribute__ ((constructor)) static void CNAME(sname)(void) \
    { \
        static struct ctest_suite suite = { \
            #sname, \
            NULL, \
            NULL, \
            CTEST_IMPL_SLIST_HEAD_INIT(suite.list), \
        }; \
        ctest_ensure_suite_hashtable_is_initialized(); \
        struct ctest_suite* const emplaced = ctest_emplace_suite(&suite); \
        emplaced->member = (ctest_fixture_func) &FNAME(sname); \
    }

#define CTEST_SETUP(sname) \
    static void CTEST_IMPL_SETUP_FNAME(sname)(struct CTEST_IMPL_DATA_SNAME(sname)* data); \
    CTEST_IMPL_FIXTURE(sname, setup, CTEST_IMPL_SETUP_FNAME, CTEST_IMPL_SETUP_CNAME) \
    static void CTEST_IMPL_SETUP_FNAME(sname)(struct CTEST_IMPL_DATA_SNAME(sname)* data)

#define CTEST_TEARDOWN(sname) \
    static void CTEST_IMPL_TEARDOWN_FNAME(sname)(struct CTEST_IMPL_DATA_SNAME(sname)* data); \
    CTEST_IMPL_FIXTURE(sname, teardown, CTEST_IMPL_TEARDOWN_FNAME, CTEST_IMPL_TEARDOWN_CNAME) \
    static void CTEST_IMPL_TEARDOWN_FNAME(sname)(struct CTEST_IMPL_DATA_SNAME(sname)* data)

And here is what the main loop might look like, with relevant changes emphasized with >>s.

        if (test == &CTEST_IMPL_TNAME(suite, test)) continue;
        if (filter(test)) {
            ctest_errorbuffer[0] = 0;
            ctest_errorsize = MSG_SIZE-1;
            ctest_errormsg = ctest_errorbuffer;
            printf("TEST %d/%d %s:%s ", idx, total, test->ssname, test->ttname);
            fflush(stdout);
            if (test->skip) {
                color_print(ANSI_BYELLOW, "[SKIPPED]");
                num_skip++;
            } else {
                int result = setjmp(ctest_err);
                if (result == 0) {
>>>>>>>>>>>>>>>>>>  suite = ctest_find_suite(test->ssname);
>>>>>>>>>>>>>>>>>>  if (suite && suite->setup && test->data) suite->setup(test->data);
                    if (test->data)
                        test->run.unary(test->data);
                    else
                        test->run.nullary();
>>>>>>>>>>>>>>>>>>  if (suite && suite->teardown && test->data) suite->teardown(test->data);
                    // if we got here it's ok
#ifdef CTEST_COLOR_OK
                    color_print(ANSI_BGREEN, "[OK]");
#else
                    printf("[OK]\n");
#endif
                    num_ok++;
                } else {
                    color_print(ANSI_BRED, "[FAIL]");
                    num_fail++;
                }
                if (ctest_errorsize != MSG_SIZE-1) printf("%s", ctest_errorbuffer);
            }
            idx++;
        }

This would add some runtime overhead, but the hashtable operations are O(1) so it shouldn't be too bad. It also has the benefit of being less clever and easier to reason about than the C tentative definition approach as well as the C++ template specialization approach.

@bvdberg
Copy link
Owner

bvdberg commented Aug 29, 2020

Could you also add a C++ version of mytests and main and add that to the Makefile. That way it's a lot easier to see if it works. Thx.

@aceckel
Copy link
Contributor Author

aceckel commented Sep 17, 2020

Could you also add a C++ version of mytests and main and add that to the Makefile. That way it's a lot easier to see if it works. Thx.

I have pushed a change that builds a test++ alongside the existing test executable by compiling main.c and mytests.c as C++ source files. Please let me know if this is sufficient or if you'd prefer that new versions of these files be checked in.

@bvdberg bvdberg merged commit cf30ad6 into bvdberg:master Sep 20, 2020
@bvdberg
Copy link
Owner

bvdberg commented Sep 20, 2020

Yes that make it easy to test. I've merged this to master, thanks!

@aceckel
Copy link
Contributor Author

aceckel commented Sep 20, 2020

Yes that make it easy to test. I've merged this to master, thanks!

Thanks for merging! However, do you have any remarks about my earlier comment #43 (comment)? I posed an alternate solution that doesn't require forking the logic for C and C++, but it is a larger architectural change. I have a functional prototype of this change (however, it is combined with the change proposed in #42 (comment)) in aceckel@b6b2cac if you are interested in seeing a working example.

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