[PATCH] D68472: [test] Depend on C.UTF-8 dependency for mri-utf8.test

Owen Reynolds via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 8 08:35:17 PDT 2019


gbreynoo added inline comments.


================
Comment at: llvm/test/tools/llvm-ar/mri-nonascii.test:15
+
+# Use input redirection to work around problems launching processess that
+# include arguments with non-ascii characters.
----------------
thopre wrote:
> MaskRay wrote:
> > hubert.reinterpretcast wrote:
> > > Minor nit: s/processess/processes/;
> > What problems do you work around? POSIX.1-2017 3.282 Portable Filename Character Set consists of the classical Latin alphabet, 0~9, <period>, <underscore>, and <hyphen-minus>. a filename consisting of the UTF-8 byte sequence 0xc2 0xa3 (£) may be disallowed by some implementations but it is unlikely that the implementation can arbitrarily reinterpret the byte sequence and cause the test to fail.
> > 
> > I suggest deleting the comment.
> The original message is not mine so I'm not sure what it referred to it might be that arguments are passed down the the program being invoked without interpretation, thus the filename would be UTF-8 encoded since that is what mri-utf8.test is encoded in. This would fail on Windows where filename must be UTF-16 and the output redirection of the earlier line would have created a filename in UTF-16.
> 
> I'll let Owen confirm.
Sorry that the comment was not clear.  The issue I had was explicitly with the behaviour differences between python versions and OS causing strings not being encoded to the right format and failing to open the file in question.

Originally I used python as opposed to filecheck as to be explicit with the expected characters and encoding. However if everyone is happier with MaskRays suggestion and it functions as expected, I'm not sure I can argue. Avoiding the dependency on the locale would be great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68472





More information about the llvm-commits mailing list