[PATCH] D95246: [SystemZ][z/OS] Fix No such file or directory expression error matching in lit tests - continued

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

I did briefly consider one alternative, which was to build these directly into FileCheck, but it doesn't quite feel like the right approach for FileCheck to need to know system level details. In practice, I think the lit substitution may be the best way forward overall. In the future, it should be easy to expand with more error messages.

You should add the new substitution to the lit command guide, so that it's documented. It might also be worth a heads up on llvm-dev to get feedback from those not directly on this review, to see if there are other ideas.



================
Comment at: llvm/utils/lit/lit/llvm/config.py:349-354
+        if (re.match(r's390x-.*-zos', triple)):
+            self.config.substitutions.append(('%err_no_such_file_or_directory', '\'EDC5129I No such file or directory.\''))
+        elif (re.match(r'.*windows.*', triple)):
+            self.config.substitutions.append(('%err_no_such_file_or_directory', '\'no such file or directory\''))
+        else:
+            self.config.substitutions.append(('%err_no_such_file_or_directory', '\'No such file or directory\''))
----------------
These lines are quite long, so probably want reflowing.

I wonder if `%errc_...` might be a better name? That way, it ties to the `std::errc` values these match up with.


================
Comment at: llvm/utils/lit/lit/llvm/config.py:369-370
 
+        if hasattr(self.config, 'host_triple'):
+           self.add_err_msg_substitutions(self.config.host_triple) 
+
----------------
Under what conditions can there not be a `host_triple`? In those cases, what happens to the tests that use the new substitution?


================
Comment at: llvm/utils/lit/lit/llvm/config.py:372
+
+
     def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
----------------
Nit: too many blank lines (compared to what was there before)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246