[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2787-2789
   // Mimicking gcc's behavior, trigraphs are only enabled if -trigraphs
-  // is specified, or -std is set to a conforming mode.
+  // is specified, -std is set to a conforming mode, or on z/OS.
   // Trigraphs are disabled by default in c++1z onwards.
----------------
If going with an integrated update to the existing sentences, both of the original sentences would require an update (not just the first one). The first sentence is also meant to describe GCC's behavior, for which I am not sure there's a statement to be made for GCC on z/OS. I suggest to add an additional sentence instead.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2791
+  Opts.Trigraphs =
+      (!Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17) || T.isOSzOS();
   Opts.Trigraphs =
----------------
I would like to point out that processing trigraphs when most platforms don't would be a portability concern. Clang appears to mitigate this somewhat with warnings.


================
Comment at: clang/test/Frontend/trigraphs.cpp:8
+// RUN: %clang_cc1 -DZOS -triple=s390x-none-zos -verify -fsyntax-only %s
+// RUN: %clang_cc1 -DZOSNOTRI -triple=s390x-none-zos -fno-trigraphs -verify -fsyntax-only %s
 
----------------
Do we know if `-fno-trigraphs` is meaningfully functional on z/OS? I believe trigraph usage might need to be replaced to use digraphs in the system headers before using `-fno-trigraphs` can be expected to work in a real user application.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85722