APValues and Constants and MaterializedTemporaryExpr need to have stable maps

[majnemer]

Summary

We risk iterator invalidation issues if we use DenseMap structures for
MaterializedTemporaryExprs. Use a std::map to ensure that they don't
move around in memory.

This fixes PR24289.

[rsmith]

include/clang/AST/ASTContext.h

180 ↗

(On Diff #30999)

I would prefer a DenseMap<const MaterializedTemporaryExpr*, APValue*>, with APValues allocated by the ASTContext's BumpPtrAllocator.

[majnemer]

• Address Justin's review comments.
• Address Richard's review comments.

[rsmith]

lib/AST/ASTContext.cpp

8557 ↗

(On Diff #32100)

Maybe use a reference here rather than a pointer.

8559–8560 ↗

(On Diff #32100)

More succinctly expressed as:

MTVI = new (*this) APValue;

8565–8566 ↗

(On Diff #32100)

This is now just MaterializedTemporaryValues.lookup(E)

lib/CodeGen/CodeGenModule.h

377 ↗

(On Diff #32100)

Can you fix this instead by not holding onto Slot in CodeGenModule::GetAddrOfGlobalTemporary? (Do the map lookup again at the end of the function.) Twice as many DenseMap lookups seems likely to be cheaper than switching to a std::map.

[majnemer]

• Address Richard's latest review comments.

[rsmith]

LGTM