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

Include test to verify patch that removes 'using namespace std' from headers #152

Closed
ekilmer opened this issue Dec 15, 2022 · 1 comment · Fixed by #155
Closed

Include test to verify patch that removes 'using namespace std' from headers #152

ekilmer opened this issue Dec 15, 2022 · 1 comment · Fixed by #155
Labels
enhancement New feature or request

Comments

@ekilmer
Copy link
Contributor

ekilmer commented Dec 15, 2022

A patch was generated (#143) to remove the using namespace std from the sleigh headers due to issues with compilation on Windows (#139).

This repo should include a test that will detect (in some way) whether the rebased patch continues to work.

Compiling the project and running the tests is a good indication that the patch continues to work, but if a new header file is added and includes a using namespace std; that we forget to remove, then we could have regressions.

It's a bit unclear to me what the test would look like, but maybe somebody has a better idea.

@ekilmer ekilmer added enhancement New feature or request help wanted Extra attention is needed labels Dec 15, 2022
@mrexodia
Copy link
Contributor

Perhaps something like this could work?

struct vector {};
struct set {};
struct unordered_map {};
struct string {};

// not sure if there are any other public headers?
#include <sleigh/libsleigh.h>

int main() {
vector v;
set s;
unordered_map m;
string str;
}

For MSVC it should use use target_compile_features(test PRIVATE cxx_std_20) because there were some weird issue with /permissive- and std::byte colliding deeply in the sleigh headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants