[PATCH] D97472: [test] Use host platform specific error message substitution in lit tests

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 4 07:41:27 PST 2021


jhenderson added a comment.

The approach basically looks fine. I haven't had a chance to verify it today. I'll try to find time tomorrow if I can.

In D97472#2601108 <https://reviews.llvm.org/D97472#2601108>, @jdenny wrote:

> Based on @jhenderson's comments, it sounds like C++ code could still call `strerror(EINVAL)` on windows.  It doesn't look like that would be handled by this patch, but I suppose we can cross that bridge if we actually come to it.

Yes, I expect that this could be a potential problem, but it's probably solved by saying "don't do that", probably in the coding standards? Perhaps worth an RFC on the mailing list to see if others have opinions. There are a limited number of existing cases that actually do call `strerror` in LLVM, but I'm guessing none of them are tested, at least not outside of a Linux environment.



================
Comment at: llvm/utils/lit/lit/llvm/config.py:351
+        host_cxx = getattr(self.config, 'host_cxx', '')
+        if (sys.platform == 'win32' and 'MSYS' not in host_cxx):
             self.config.substitutions.append(('%errc_ENOENT', '\'no such file or directory\''))
----------------
Add a comment here explaining why we have to treat windows behaviour separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97472



More information about the llvm-commits mailing list