[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2639481 , @zero9178 wrote:

> Changes in this patch are based on this one https://reviews.llvm.org/D97472. 
> In the discussion there it was deemed not a good solution to use case 
> insensitive comparison as that would make any other matches case insensitive 
> as well, which might be a source of bugs.



> Prior to this patch it worke with an MSVC configuration already as the 
> strings were correctly hardcoded for the MSVC STL. Problem was when using any 
> other compiler configuration on Windows. In my case I am using your 
> llvm-mingw distribution to build all of LLVM and since it uses libc++ it 
> produces different error messages from the one in MSVC STL. I observed the 
> same problem when using GCC on Windows and if one were to theoretically use 
> libc++ with clang-cl in a MSVC environment, tests would be failing as well 
> due to a mismatch in error strings.

Ok, I think I see - so longterm (i.e. up until to a few months ago), we've just 
had some hardcoded (case insensitive) regexes in some testcases, and these 
didn't match for z/OS. As part of efforts to make it work for z/OS, a number of 
different incarnations have been used, and using a build-time tool to extract 
the real message looks like the most flexible solution in the end. And that 
also allows making it work properly for cases on Windows, where it's hard to 
know exactly which stdlib is going to be used at runtime and which kind of 
messages it produces.

Ok, so that makes sense to me, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D98278#2638005 , @mstorsjo wrote:

> In D98278#2637866 , @zero9178 wrote:
>
>> In D98278#2637826 , @mstorsjo wrote:
>>
>>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>>> where e.g. some translations produce different error messages?
>>
>> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
>> the case of MSVCs STL, the messages from `std::error_codes` which are used 
>> by various LLVM tools produce different strings then using `strerror` (the C 
>> function also called by Python) with the same error codes (Specifically, it 
>> has different casing).
>
> Ok, but would e.g. a case insensitive comparison have worked instead of this?
>
> And didn't the python script have hardcoded strings, specifically for the 
> MSVC case? Why weren't they written with the right casing for the case that 
> they're supposed to match? I.e. was it an issue with the existing hardcoded 
> strings, or did they work in one case but not another one?

Changes in this patch are based on this one https://reviews.llvm.org/D97472. In 
the discussion there it was deemed not a good solution to use case insensitive 
comparison as that would make any other matches case insensitive as well, which 
might be a source of bugs.

In D98278#2638023 , @mstorsjo wrote:

> In D98278#2637866 , @zero9178 wrote:
>
>> In D98278#2637826 , @mstorsjo wrote:
>>
>>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>>> where e.g. some translations produce different error messages?
>>
>> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
>> the case of MSVCs STL, the messages from `std::error_codes` which are used 
>> by various LLVM tools produce different strings then using `strerror` (the C 
>> function also called by Python) with the same error codes (Specifically, it 
>> has different casing).
>
> Or turning the question another way around: We have a couple different bots 
> that build and run the tests, successfully, with MSVC configurations. Are 
> there tests that failed for you in your configuration, that succeed in the 
> setup of the bots? Or are there other tests that aren't run as part of bots 
> that you're fixing? It's all still very vague to me.

Prior to this patch it worke with an MSVC configuration already as the strings 
were correctly hardcoded for the MSVC STL. Problem was when using any other 
compiler configuration on Windows. In my case I am using your llvm-mingw 
distribution to build all of LLVM and since it uses libc++ it produces 
different error messages from the one in MSVC STL. I observed the same problem 
when using GCC on Windows and if one were to theoretically use libc++ with 
clang-cl in a MSVC environment, tests would be failing as well due to a 
mismatch in error strings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2637866 , @zero9178 wrote:

> In D98278#2637826 , @mstorsjo wrote:
>
>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>> where e.g. some translations produce different error messages?
>
> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
> the case of MSVCs STL, the messages from `std::error_codes` which are used by 
> various LLVM tools produce different strings then using `strerror` (the C 
> function also called by Python) with the same error codes (Specifically, it 
> has different casing).

Or turning the question another way around: We have a couple different bots 
that build and run the tests, successfully, with MSVC configurations. Are there 
tests that failed for you in your configuration, that succeed in the setup of 
the bots? Or are there other tests that aren't run as part of bots that you're 
fixing? It's all still very vague to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2637866 , @zero9178 wrote:

> In D98278#2637826 , @mstorsjo wrote:
>
>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>> where e.g. some translations produce different error messages?
>
> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
> the case of MSVCs STL, the messages from `std::error_codes` which are used by 
> various LLVM tools produce different strings then using `strerror` (the C 
> function also called by Python) with the same error codes (Specifically, it 
> has different casing).

Ok, but would e.g. a case insensitive comparison have worked instead of this?

And didn't the python script have hardcoded strings, specifically for the MSVC 
case? Why weren't they written with the right casing for the case that they're 
supposed to match? I.e. was it an issue with the existing hardcoded strings, or 
did they work in one case but not another one?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D98278#2637826 , @mstorsjo wrote:

> Btw, while this change does explain _what_ it does, it doesn't actually say 
> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
> where e.g. some translations produce different error messages?

Now that you mention it, it's indeed not as clear as I thought. But yes, in the 
case of MSVCs STL, the messages from `std::error_codes` which are used by 
various LLVM tools produce different strings then using `strerror` (the C 
function also called by Python) with the same error codes (Specifically, it has 
different casing).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Btw, while this change does explain _what_ it does, it doesn't actually say the 
exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case where 
e.g. some translations produce different error messages?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D98278#2635190 , @ashi1 wrote:

> In D98278#2618936 , @zero9178 wrote:
>
>> In D98278#2616932 , @mstorsjo wrote:
>>
>>> In D98278#2616916 , @zero9178 
>>> wrote:
>>>
 Add GetErrcMessages.cmake, which contains a cmake function to 
 automatically get the error messages of various posix error codes needed 
 by lit by running a small C++ program.
 Currently ENOENT, EISDIR, EINVAL and EACCES are supplied. 
 These error messages are then currently supplied to clang, llvm and lld as 
 the errc_messages config parameter.

 Regarding Cross compiling: the function uses try_run which when cross 
 compiling may use the CMAKE_CROSSCOMPILING_EMULATOR to run the code.
>>>
>>> How does it behave if such a thing isn't hooked up? Ideally it'd fall back 
>>> silently and these parts of tests would just fail, but not block things 
>>> overall.
>>
>> It will fall back to using Python's strerror, potentially failing if pythons 
>> strerror would not return the same strings (only the case for MSVC I 
>> believe).
>
> Hi, this patch is causing a failure when cmake runs `try_run()` with a 
> `CMAKE_TOOLCHAIN_FILE` defined. It seems related to having a fall back if the 
> try_run fails.
> Here is the error, and the build uses MSVC. What do you think we could do 
> about this? Thanks.
>
>   [06:48:06][Step 1/4] CMake Error: TRY_RUN() invoked in cross-compiling 
> mode, please set the following cache variables appropriately:
>   [06:48:06][Step 1/4]errc_exit_code (advanced)
>   [06:48:06][Step 1/4]errc_exit_code__TRYRUN_OUTPUT (advanced)
>   [06:48:06][Step 1/4] For details see /TryRunResults.cmake

Hello! There is actually an ongoing discussion about this here 
https://reviews.llvm.org/D98861. For the time being you have two options if you 
have a cross compiling setup that can't natively run Windows programs.

1. Set CMAKE_CROSSCOMPILING_EMULATOR to an emulator that could execute the 
Windows program on your host. Wine comes to my mind as an example
2. You should be able to set the variables `errc_exit_code` to the value 0 and 
then set `errc_exit_code__TRYRUN_OUTPUT` to the stdout of the Windows program 
(alternatively leave it empty, but a few tests will fail)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added a comment.

In D98278#2618936 , @zero9178 wrote:

> In D98278#2616932 , @mstorsjo wrote:
>
>> In D98278#2616916 , @zero9178 wrote:
>>
>>> Add GetErrcMessages.cmake, which contains a cmake function to automatically 
>>> get the error messages of various posix error codes needed by lit by 
>>> running a small C++ program.
>>> Currently ENOENT, EISDIR, EINVAL and EACCES are supplied. 
>>> These error messages are then currently supplied to clang, llvm and lld as 
>>> the errc_messages config parameter.
>>>
>>> Regarding Cross compiling: the function uses try_run which when cross 
>>> compiling may use the CMAKE_CROSSCOMPILING_EMULATOR to run the code.
>>
>> How does it behave if such a thing isn't hooked up? Ideally it'd fall back 
>> silently and these parts of tests would just fail, but not block things 
>> overall.
>
> It will fall back to using Python's strerror, potentially failing if pythons 
> strerror would not return the same strings (only the case for MSVC I believe).

Hi, this patch is causing a failure when cmake runs `try_run()` with a 
`CMAKE_TOOLCHAIN_FILE` defined. It seems related to having a fall back if the 
try_run fails.
Here is the error, and the build uses MSVC. What do you think we could do about 
this? Thanks.

  [06:48:06][Step 1/4] CMake Error: TRY_RUN() invoked in cross-compiling mode, 
please set the following cache variables appropriately:
  [06:48:06][Step 1/4]errc_exit_code (advanced)
  [06:48:06][Step 1/4]errc_exit_code__TRYRUN_OUTPUT (advanced)
  [06:48:06][Step 1/4] For details see /TryRunResults.cmake


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D98278#2628527 , @uabelho wrote:

> In D98278#2628482 , @zero9178 wrote:
>
>> In D98278#2628477 , @uabelho wrote:
>>
>>> so perhaps there should be some additional error handling when running the 
>>> compiled program doesn't work?
>>
>> Could you try changing line 32 of llvm/cmake/modules/GetErrcMessages.cmake 
>> from `if (errc_compiled)` to `if (errc_compiled AND "${errc_exit_code}" 
>> STREQUAL "0")` and report back if it changes anything? I think that should 
>> fix your issue, although it'll fall back to using python's strerror 
>> messages. Indeed an oversight of mine.
>
> Yes that helps, please submit. Thanks!

Pushed in https://reviews.llvm.org/rG953bb5e5c8f60dc769942a3615d800fe166ffd1d


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D98278#2628482 , @zero9178 wrote:

> In D98278#2628477 , @uabelho wrote:
>
>> so perhaps there should be some additional error handling when running the 
>> compiled program doesn't work?
>
> Could you try changing line 32 of llvm/cmake/modules/GetErrcMessages.cmake 
> from `if (errc_compiled)` to `if (errc_compiled AND "${errc_exit_code}" 
> STREQUAL "0")` and report back if it changes anything? I think that should 
> fix your issue, although it'll fall back to using python's strerror messages. 
> Indeed an oversight of mine.

Yes that helps, please submit. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D98278#2628477 , @uabelho wrote:

> Hi,
>
> I'm seeing a problem with this. Compiling with gcc 9.3.0 the compilation of 
> the test program works, but then when I run it I get
>
>   
> /repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188:
>  /lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by 
> /repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188)
>
> and this causes everything to fail with
>
>   llvm-lit: 
> /repo/uabelho/master-github/llvm/utils/lit/lit/TestingConfig.py:100: fatal: 
> unable to parse config file 
> '/repo/uabelho/master-github/llvm/build-all-bbigcc/tools/clang/test/lit.site.cfg.py',
>  traceback: Traceback (most recent call last):
> File 
> "/repo/uabelho/master-github/llvm/build-all-bbigcc/bin/../../utils/lit/lit/TestingConfig.py",
>  line 89, in load_from_path
>   exec(compile(data, path, 'exec'), cfg_globals, None)
> File 
> "/repo/uabelho/master-github/llvm/build-all-bbigcc/tools/clang/test/lit.site.cfg.py",
>  line 19
>   config.errc_messages = 
> "/repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188:
>  /lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by 
> /repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188)
>
> so perhaps there should be some additional error handling when running the 
> compiled program doesn't work?

Could you try changing line 32 of llvm/cmake/modules/GetErrcMessages.cmake from 
`if (errc_compiled)` to `if (errc_compiled AND "${errc_test_code}" STREQUAL 
"0")` and report back if it changes anything? I think that should fix your 
issue, although it'll fall back to using python's strerror messages. Indeed an 
oversight of mine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hi,

I'm seeing a problem with this. Compiling with gcc 9.3.0 the compilation of the 
test program works, but then when I run it I get

  
/repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188:
 /lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by 
/repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188)

and this causes everything to fail with

  llvm-lit: 
/repo/uabelho/master-github/llvm/utils/lit/lit/TestingConfig.py:100: fatal: 
unable to parse config file 
'/repo/uabelho/master-github/llvm/build-all-bbigcc/tools/clang/test/lit.site.cfg.py',
 traceback: Traceback (most recent call last):
File 
"/repo/uabelho/master-github/llvm/build-all-bbigcc/bin/../../utils/lit/lit/TestingConfig.py",
 line 89, in load_from_path
  exec(compile(data, path, 'exec'), cfg_globals, None)
File 
"/repo/uabelho/master-github/llvm/build-all-bbigcc/tools/clang/test/lit.site.cfg.py",
 line 19
  config.errc_messages = 
"/repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188:
 /lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by 
/repo/uabelho/master-github/llvm/build-all-bbigcc/CMakeFiles/CMakeTmp/cmTC_00188)

so perhaps there should be some additional error handling when running the 
compiled program doesn't work?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

In D98278#2628433 , @zero9178 wrote:

> I addressed your comments in 
> https://reviews.llvm.org/rG4a17ac0387f078529da02e355a24df99f645d364. Hope it 
> should be alright now

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 marked 4 inline comments as done.
zero9178 added a comment.

I addressed your comments in 
https://reviews.llvm.org/rG4a17ac0387f078529da02e355a24df99f645d364. Hope it 
should be alright now


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[James Henderson via Phabricator via cfe-commits]

jhenderson added inline comments.



Comment at: llvm/cmake/modules/GetErrcMessages.cmake:3-6
+# The purpose of this function is to supply those error messages to llvm-lit 
using the errc_messages config
+# Currently supplied and needed error codes: ENOENT, EISDIR, EINVAL and EACCES
+# Messages are semi colon separated
+# Keep amount, order and tested error codes in sync with 
llvm/utils/lit/lit/llvm/config.py

zero9178 wrote:
> jhenderson wrote:
> > Each of these lines look like they need a trailing full stop added.
> Just to double check, that means adding a `.` at the end of the lines right?
Yes, that's right. Full stop (British English) == period (American English) == 
`.`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 marked an inline comment as done.
zero9178 added inline comments.



Comment at: llvm/cmake/modules/GetErrcMessages.cmake:3-6
+# The purpose of this function is to supply those error messages to llvm-lit 
using the errc_messages config
+# Currently supplied and needed error codes: ENOENT, EISDIR, EINVAL and EACCES
+# Messages are semi colon separated
+# Keep amount, order and tested error codes in sync with 
llvm/utils/lit/lit/llvm/config.py

jhenderson wrote:
> Each of these lines look like they need a trailing full stop added.
Just to double check, that means adding a `.` at the end of the lines right?



Comment at: llvm/utils/lit/lit/llvm/config.py:14
 lit_path_displayed = False
+python_errc_displayed = False
 

jhenderson wrote:
> Seems like this variable is unused and got leftover from an experiment at 
> some point?
Correct, that was accidently left in. I immediately followed up with a commit 
removing it here: 
https://reviews.llvm.org/rG68e4084bf68ae7517491a6a0cbfd6d3b6f93cef7


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Thanks for working on this. A couple of post-commit comments to look at, 
otherwise looks good. Thanks for figuring out how to do this in a portable 
manner!




Comment at: llvm/cmake/modules/GetErrcMessages.cmake:1
+
+# This function returns the messages of various POSIX error codes as they are 
returned by std::error_code.

Delete this blank line?



Comment at: llvm/cmake/modules/GetErrcMessages.cmake:3-6
+# The purpose of this function is to supply those error messages to llvm-lit 
using the errc_messages config
+# Currently supplied and needed error codes: ENOENT, EISDIR, EINVAL and EACCES
+# Messages are semi colon separated
+# Keep amount, order and tested error codes in sync with 
llvm/utils/lit/lit/llvm/config.py

Each of these lines look like they need a trailing full stop added.



Comment at: llvm/utils/lit/lit/llvm/config.py:14
 lit_path_displayed = False
+python_errc_displayed = False
 

Seems like this variable is unused and got leftover from an experiment at some 
point?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf2796c76d2f: [test] Add ability to get error messages from 
CMake for errc substitution (authored by zero9178).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D98278?vs=330653&id=330769#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98278/new/

https://reviews.llvm.org/D98278

Files:
  clang/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  lld/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  llvm/CMakeLists.txt
  llvm/cmake/modules/GetErrcMessages.cmake
  llvm/test/lit.site.cfg.py.in
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -11,6 +11,7 @@
 from lit.llvm.subst import ToolSubst
 
 lit_path_displayed = False
+python_errc_displayed = False
 
 class LLVMConfig(object):
 
@@ -356,19 +357,24 @@
 return True
 
 def add_err_msg_substitutions(self):
-host_cxx = getattr(self.config, 'host_cxx', '')
-# On Windows, python's os.strerror() does not emit the same spelling as
-# the C++ std::error_code.  As a workaround, hardcode the Windows error
-# message.
-if (sys.platform == 'win32' and 'MSYS' not in host_cxx):
+# Python's strerror may not supply the same message
+# as C++ std::error_code. One example of such a platform is
+# Visual Studio. errc_messages may be supplied which contains the error
+# messages for ENOENT, EISDIR, EINVAL and EACCES as a semi colon
+# separated string. LLVM testsuites can use get_errc_messages in cmake
+# to automatically get the messages and pass them into lit.
+errc_messages = getattr(self.config, 'errc_messages', '')
+if len(errc_messages) != 0:
+(errc_enoent, errc_eisdir,
+ errc_einval, errc_eacces) = errc_messages.split(';')
 self.config.substitutions.append(
-('%errc_ENOENT', '\'no such file or directory\''))
+('%errc_ENOENT', '\'' + errc_enoent + '\''))
 self.config.substitutions.append(
-('%errc_EISDIR', '\'is a directory\''))
+('%errc_EISDIR', '\'' + errc_eisdir + '\''))
 self.config.substitutions.append(
-('%errc_EINVAL', '\'invalid argument\''))
+('%errc_EINVAL', '\'' + errc_einval + '\''))
 self.config.substitutions.append(
-('%errc_EACCES', '\'permission denied\''))
+('%errc_EACCES', '\'' + errc_eacces + '\''))
 else:
 self.config.substitutions.append(
 ('%errc_ENOENT', '\'' + os.strerror(errno.ENOENT) + '\''))
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -12,6 +12,7 @@
 config.llvm_shlib_ext = "@SHLIBEXT@"
 config.llvm_exe_ext = "@EXEEXT@"
 config.lit_tools_dir = path(r"@LLVM_LIT_TOOLS_DIR@")
+config.errc_messages = "@LLVM_LIT_ERRC_MESSAGES@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.gold_executable = "@GOLD_EXECUTABLE@"
 config.ld64_executable = "@LD64_EXECUTABLE@"
Index: llvm/cmake/modules/GetErrcMessages.cmake
===
--- /dev/null
+++ llvm/cmake/modules/GetErrcMessages.cmake
@@ -0,0 +1,39 @@
+
+# This function returns the messages of various POSIX error codes as they are returned by std::error_code.
+# The purpose of this function is to supply those error messages to llvm-lit using the errc_messages config
+# Currently supplied and needed error codes: ENOENT, EISDIR, EINVAL and EACCES
+# Messages are semi colon separated
+# Keep amount, order and tested error codes in sync with llvm/utils/lit/lit/llvm/config.py
+function(get_errc_messages outvar)
+
+set(errc_test_code ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/getErrc.cpp)
+
+file(WRITE ${errc_test_code} "
+#include 
+#include 
+#include 
+#include 
+
+std::string getMessageFor(int err) {
+return std::make_error_code(static_cast(err)).message();
+}
+
+int main() {
+std::cout << getMessageFor(ENOENT) << ';' << getMessageFor(EISDIR);
+std::cout << ';' << getMessageFor(EINVAL) << ';' << getMessageFor(EACCES);
+}
+")
+
+try_run(errc_exit_code
+errc_compiled
+${CMAKE_BINARY_DIR}
+${errc_test_code}
+RUN_OUTPUT_VARIABLE errc_result
+COMPILE_OUTPUT_VARIABLE errc_compile_errors)
+if (errc_compiled)
+set(${outvar} ${errc_result} PARENT_SCOPE