Re: [PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

2017-08-30 Thread Richard Smith via cfe-commits 
On 30 August 2017 at 11:52, Boris Kolpackov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Victor Leschuk  writes:
>
> > Hello Boris, looks like this revision broke tests on our win10 builder:
> > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_
> 64-scei-ps4-windows10pro-fast/builds/11760
> >
> > Clang :: CXX/modules-ts/basic/basic.link/module-declaration.cpp
> >
> > I had to revert this revision. Could you please take a look?
>
> Sorry about that.
>
> I took a look and it seems the problem is with the test's path regex:
>
> '{{.*}}/x.pcm'
>
> Which, on Windows, is being matched against a path like this:
>
> ...\module-declaration.cpp.tmp\x.pcm
>
> It looks like the "canonical" fix for this is to use [/\\].
>
> What is the protocol in this situation? Should I fix the test and then
> re-apply my patch?


Yes, please.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

2017-08-30 Thread Boris Kolpackov via cfe-commits 
Victor Leschuk  writes:

> Hello Boris, looks like this revision broke tests on our win10 builder:
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/11760
> 
> Clang :: CXX/modules-ts/basic/basic.link/module-declaration.cpp
> 
> I had to revert this revision. Could you please take a look?

Sorry about that.

I took a look and it seems the problem is with the test's path regex:

'{{.*}}/x.pcm'

Which, on Windows, is being matched against a path like this:

...\module-declaration.cpp.tmp\x.pcm

It looks like the "canonical" fix for this is to use [/\\].

What is the protocol in this situation? Should I fix the test and then
re-apply my patch?

Thanks,
Boris
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

2017-08-30 Thread Victor Leschuk via cfe-commits 
Hello Boris, looks like this revision broke tests on our win10 builder:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/11760

Clang :: CXX/modules-ts/basic/basic.link/module-declaration.cpp

I had to revert this revision. Could you please take a look?


On 08/30/2017 11:47 AM, Phabricator via Phabricator via cfe-commits wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL312105: [modules] Add ability to specify module name to 
> module file mapping (authored by borisk).
>
> Changed prior to commit:
>   https://reviews.llvm.org/D35020?vs=111826=113207#toc
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D35020
>
> Files:
>   cfe/trunk/docs/Modules.rst
>   cfe/trunk/include/clang/Driver/Options.td
>   cfe/trunk/include/clang/Lex/HeaderSearch.h
>   cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
>   cfe/trunk/include/clang/Serialization/ASTReader.h
>   cfe/trunk/include/clang/Serialization/ModuleManager.h
>   cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>   cfe/trunk/lib/Frontend/CompilerInstance.cpp
>   cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>   cfe/trunk/lib/Frontend/FrontendActions.cpp
>   cfe/trunk/lib/Lex/HeaderSearch.cpp
>   cfe/trunk/lib/Serialization/ASTReader.cpp
>   cfe/trunk/lib/Serialization/ASTWriter.cpp
>   cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
>   cfe/trunk/lib/Serialization/ModuleManager.cpp
>   cfe/trunk/test/CXX/modules-ts/basic/basic.search/module-import.cpp
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-- 
Best Regards,

Victor Leschuk | Software Engineer |Access Softek

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits