Skip to content

add _ENFORCE_ONLY_CORE_HEADERS#2068

Merged
StephanTLavavej merged 22 commits intomicrosoft:mainfrom
fsb4000:fix311
Aug 31, 2022
Merged

add _ENFORCE_ONLY_CORE_HEADERS#2068
StephanTLavavej merged 22 commits intomicrosoft:mainfrom
fsb4000:fix311

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 and others added 3 commits July 21, 2021 22:33
Co-authored-by: Michael Schellenberger Costa <mschellenbergercosta@gmail.com>
@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