Skip to content

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Jul 21, 2021

Fixes #311

These are probably terrible error messages.
Feel free to suggest a better :)

As far as I remember, I already asked. But I still want to clarify. We can't create a failing test. They are only available in the LLVM test suite, right?

@fsb4000 fsb4000 requested a review from a team as a code owner July 21, 2021 15:12
@miscco
Copy link
Contributor

miscco commented Jul 21, 2021

We are using lit to run the test. Did you try a test.compile.fail.cpp

@fsb4000
Copy link
Contributor Author

fsb4000 commented Jul 21, 2021

Did you try a test.compile.fail.cpp

no.

Can I have 2 failing tests in 1 folder?

One for _FORBID_ALL_STL_HEADERS, one for _ENFORCE_ONLY_CORE_HEADERS?

@miscco
Copy link
Contributor

miscco commented Jul 21, 2021

I do not know, but you can always just add two folders i guess

@fsb4000
Copy link
Contributor Author

fsb4000 commented Jul 21, 2021

No. It doesn't work.

When I have test.compile.fail.cpp, I get:

python tests\utils\stl-lit\stl-lit.py ..\..\..\tests\std\tests\GH_000311_enforce_core_headers
stl-lit.py: C:\Dev\STL\llvm-project\llvm\utils\lit\lit\discovery.py:267: warning: input '..\\..\\..\\tests\\std\\tests\\GH_000311_enforce_core_headers' contained no tests
error: did not discover any tests for provided path(s)

@AdamBucior
Copy link
Contributor

We need to verify that all non-core headers include yvals.h instead of yvals_core.h.

#include <yvals_core.h>

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Jul 21, 2021
@fsb4000
Copy link
Contributor Author

fsb4000 commented Jul 21, 2021

@miscco I found two links:

config.suffixes = ['test[.]cpp$', 'test[.]compile[.]pass[.]cpp$']

https://github.com/llvm/llvm-project/blob/4e52a04833fb352090498d6d1c013a2c61d75e53/libcxx/test/configs/legacy.cfg.in#L45

So with default configuration we can't create a failing test. But I think it's possible with a custom python script...

@StephanTLavavej
Copy link
Member

We made a decision on #311 in the weekly maintainer meeting - let's keep _ENFORCE_ONLY_CORE_HEADERS but drop the _FORBID_ALL_STL_HEADERS functionality, since we haven't found a compelling user scenario for that.

@StephanTLavavej StephanTLavavej self-assigned this Jul 21, 2021
@StephanTLavavej StephanTLavavej changed the title add _ENFORCE_ONLY_CORE_HEADERS and _FORBID_ALL_STL_HEADERS add _ENFORCE_ONLY_CORE_HEADERS May 18, 2022
@StephanTLavavej
Copy link
Member

Thanks! (And apologies for the extreme delay here. 🙀) I've pushed changes to use the new STL error message machinery, to perform this check for only real compilers (non-compiler tools have "preprocessors" with sometimes questionable behavior), and to fuse the test coverage into the pre-existing test for core headers where this makes perfect sense.

Users will see:

D:\GitHub\STL>type meow.cpp
#include <vector>
int main() {}

D:\GitHub\STL>cl /EHsc /nologo /W4 /D_ENFORCE_ONLY_CORE_HEADERS meow.cpp
meow.cpp
D:\GitHub\STL\out\build\x64\out\inc\yvals.h(14): error STL1005: Tried to include a non-core C++ Standard Library header file with _ENFORCE_ONLY_CORE_HEADERS defined.
D:\GitHub\STL\out\build\x64\out\inc\yvals.h(13): error C2338: static_assert failed: 'Error in C++ Standard Library usage.'

@StephanTLavavej
Copy link
Member

I'm speculatively mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit de060c5 into microsoft:main Aug 31, 2022
@StephanTLavavej
Copy link
Member

Thanks for making it easier for users to limit themselves to the core subset! 🎯 ✅ 🎉

@fsb4000 fsb4000 deleted the fix311 branch September 1, 2022 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it easy to limit the STL to core features
5 participants