[PATCH] libc++: Add option to disable access to the global filesystem namespace

Jonathan Roelofs jonathan at codesourcery.com
Tue Mar 10 10:11:01 PDT 2015



On 3/10/15 10:02 AM, Ed Schouten wrote:
> In http://reviews.llvm.org/D8194#137640, @jroelofs wrote:
>
>> Seems reasonable to me. I think you ought to fix a few of the tests that this "breaks", and add a few *.fail.cpp ones to verify that the functions you expect to be removed have actually been removed. For example ./test/std/input.output/file.streams/c.files/cstdio.pass.cpp should have:
>>
>>    static_assert((std::is_same<decltype(std::fflush(fp)), int>::value), "");
>>    #if defined(_LIBCPP_HAS_GLOBAL_FILESYSTEM_NAMESPACE)
>>    static_assert((std::is_same<decltype(std::fopen("", "")), std::FILE*>::value), "");
>>    static_assert((std::is_same<decltype(std::freopen("", "", fp)), std::FILE*>::value), "");
>>    #endif
>>    static_assert((std::is_same<decltype(std::setbuf(fp,cp)), void>::value), "");
>>
>>
>> And then I'd add a ./test/std/input.output/file.streams/c.files/no.global.filesystem.namespace/fopen.fail.cpp with:
>>
>>    // REQUIRES: CloudABI
>>
>>    #include <cstdio>
>>
>>    int main() {
>>        // fopen is not allowed on systems without the global filesystem namespace
>>        std::fopen("", "");
>>    }
>
>
> Yes, exactly! I was planning on doing something similar to what you've described. Good to see that I'm starting to get the hang of it.
:D
>
> So my current roadmap is to first get libc++ to build on CloudABI before I attempt to tackle the test suite. In my opinion it doesn't make sense to patch up the test suite if the tests cannot be run decently. Being able to build libc++ would be a prerequisite. Getting all the tests to pass followed by setting up a buildbot will be my next milestones.
>
> Does that approach sound reasonable to you?

That vaguely reflects how I have been going about adding 
baremetal+Newlib support. Where I was able to, I tried to commit test 
cases with the patches, though a bit of that was tough because there 
wasn't yet remote testing support in the lit config at the time (I had 
been using a hacked up version of testit... eww).

In the case of this particular patch, if you invert the name and 
semantics of the guard to _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE, 
then you can test these changes on platforms that do in fact have the 
global filesystem namespace, like I've done with _LIBCPP_HAS_NO_THREADS. 
See 
https://github.com/llvm-mirror/libcxx/blob/master/test/libcxx/test/config.py#L399 
and 
http://lab.llvm.org:8011/builders/libcxx-libcxxabi-singlethreaded-x86_64-linux-debian


Cheers,

Jon

>
> Thanks,
> Ed
>
>
> REPOSITORY
>    rL LLVM
>
> http://reviews.llvm.org/D8194
>
> EMAIL PREFERENCES
>    http://reviews.llvm.org/settings/panel/emailpreferences/
>
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded



More information about the cfe-commits mailing list