Add back include for AutoConvert.h as it's needed for z/OS

[perry-ca]

The commit a1935fd removed an include that is needed when building on z/OS.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Sean Perry (perry-ca)

Changes

The commit a1935fd removed an include that is needed when building on z/OS.

---

Full diff: https://github.com/llvm/llvm-project/pull/135430.diff

1 Files Affected:

• (modified) clang/lib/Basic/SourceManager.cpp (+1)

diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index a78ffc1e90ebe..6d6e54b1bec69 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -24,6 +24,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Allocator.h"
+#include "llvm/Support/AutoConvert.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Endian.h"

[abhina-sree]

LGTM

[zibi2]

LGTM

[rnk]

I just happened to look into the implementation of AutoConvert.h, and I see the entire interface is defined away when not targeting zOS. This means that all call sites need to be conditional on ifdef __MVS__, which means we have ugly ifdef droppings all over the codebase, which harms readability and understandability.

Can whoever owns this redesign the header so that it provides inline functions which have default implementations that return false / no_error to disable their functionality on non-zOS platforms? Otherwise this failure mode will happen again in the future. This seems like feedback that should've come up in the original review (https://reviews.llvm.org/D100483), but I don't recognize enough of the contributor names there to confirm that there was some approval from maintainers or the broader community, it looks like this was mostly approved by zOS stakeholders.

[perry-ca]

I just happened to look into the implementation of AutoConvert.h, and I see the entire interface is defined away when not targeting zOS. This means that all call sites need to be conditional on ifdef __MVS__, which means we have ugly ifdef droppings all over the codebase, which harms readability and understandability.

Can whoever owns this redesign the header so that it provides inline functions which have default implementations that return false / no_error to disable their functionality on non-zOS platforms? Otherwise this failure mode will happen again in the future. This seems like feedback that should've come up in the original review (https://reviews.llvm.org/D100483), but I don't recognize enough of the contributor names there to confirm that there was some approval from maintainers or the broader community, it looks like this was mostly approved by zOS stakeholders.

I had the same reflection. I'd get that change made. Thanks