[llvm] Input iterators should not const-overload `operator*` with varying return types

[StephanTLavavej]

The C++ Standard requires that input (and stronger) iterators return the same type when dereferenced, regardless of the top-level constness of the iterator. This is N4993 [iterator.cpp17.general]/1:

In the following sections, a and b denote values of type X or const X, difference_type and reference refer to the types iterator_traits<X>::difference_type and iterator_traits<X>::reference, respectively

followed by [tab:inputiterator]:

Expression: *a
Return type: reference, convertible to T

In MSVC's STL, we recently merged microsoft/STL#5135, which innocently added top-level constness to an iterator before dereferencing it, and that broke LLVM's build when this iterator was passed to std::uninitialized_copy:

llvm-project/llvm/include/llvm/IR/DebugProgramInstruction.h

Lines 373 to 379 in 342c8db

	const Value *operator*() const {
	ValueAsMetadata *VAM = isa<ValueAsMetadata *>(I)
	? cast<ValueAsMetadata *>(I)
	: *cast<ValueAsMetadata **>(I);
	return VAM->getValue();
	};
	Value *operator*() {

LLVM's iterator should not be overloading const Value *operator*() const and Value *operator*() with a varying return type like this. I searched this directory, and found a very similar occurrence:

llvm-project/llvm/include/llvm/IR/IntrinsicInst.h

Lines 212 to 218 in 342c8db

	const Value *operator*() const {
	ValueAsMetadata *VAM = isa<ValueAsMetadata *>(I)
	? cast<ValueAsMetadata *>(I)
	: *cast<ValueAsMetadata **>(I);
	return VAM->getValue();
	};
	Value *operator*() {

And more potentially affected occurrences:

llvm-project/llvm/include/llvm/ADT/DenseSet.h

Lines 120 to 121 in 342c8db

	ValueT &operator*() { return I->getFirst(); }
	const ValueT &operator*() const { return I->getFirst(); }

llvm-project/llvm/include/llvm/ADT/StringExtras.h

Lines 588 to 590 in 342c8db

	const StringRef &operator*() const { return Current; }
	StringRef &operator*() { return Current; }

llvm-project/llvm/include/llvm/DebugInfo/PDB/Native/DbiModuleList.h

Lines 46 to 47 in 342c8db

	const StringRef &operator*() const { return ThisValue; }
	StringRef &operator*() { return ThisValue; }

llvm-project/llvm/include/llvm/Support/Error.h

Lines 628 to 634 in 342c8db

	reference operator*() {
	assertIsChecked();
	return *getStorage();
	}
	/// Returns a const reference to the stored T value.
	const_reference operator*() const {

llvm-project/llvm/include/llvm/Support/ManagedStatic.h

Lines 86 to 96 in 342c8db

	C &operator*() {
	void *Tmp = Ptr.load(std::memory_order_acquire);
	if (!Tmp)
	RegisterManagedStatic(Creator::call, Deleter::call);
	return *static_cast<C *>(Ptr.load(std::memory_order_relaxed));
	}
	C *operator->() { return &**this; }
	const C &operator*() const {

I'm probably going to have to revert this specific change of adding top-level constness within MSVC's STL, but I wanted to let you know about these non-conforming LLVM iterators.

[CaseyCarter]

C *operator->() { return &**this; } is a related code smell in

llvm-project/llvm/include/llvm/Support/ManagedStatic.h

Line 94 in 342c8db

C *operator->() { return &**this; }

For an input (or stronger) iterator, the behavior of operator->() and operator->() const cannot differ, so there's generally no non-const overload.