This is an archive of the discontinued LLVM Phabricator instance.

AST: Mangle reference temporaries reliably
ClosedPublic

Authored by majnemer on Apr 29 2014, 5:26 PM.

Details

Summary

Previously, we would generate a single name for all reference
temporaries and allow LLVM to rename them for us. Instead, number the
reference temporaries as we build them in Sema.

Diff Detail

Event Timeline

majnemer updated this revision to Diff 8947.Apr 29 2014, 5:26 PM
majnemer retitled this revision from to AST: Mangle reference temporaries reliably.
majnemer updated this object.
majnemer added a reviewer: rsmith.
majnemer added a subscriber: Unknown Object (MLST).
rsmith edited edge metadata.Apr 29 2014, 5:58 PM

Please also propose this mangling scheme on cxx-abi-dev.

lib/AST/ExprCXX.cpp
1458–1459
auto ES = new (ExtendedBy->getASTContext()) ExtraState;
1464–1465

Maybe stash the ExtraState* in a variable to avoid repeateding geting it here?

lib/Sema/SemaInit.cpp
5767–5771

It's not sufficient to start the numbering from 1 each time: we need to number within the topmost InitializedEntity, not just within the subentity for which we're currently building an initializer. Example:

struct S { const int &a, const int &b; };
S s = { 1, 2 };
lib/Serialization/ASTWriterStmt.cpp
1569–1573

We usually serialize fields in the order they appear in the class (so mangling number would go last). Not a big deal, though.

majnemer updated this revision to Diff 8951.Apr 29 2014, 9:24 PM
majnemer edited edge metadata.

Address Richard's review comments

rsmith added inline comments.Apr 30 2014, 7:02 PM
include/clang/Sema/Initialization.h
424

Maybe make this allocate a mangling number and return it, rather than returning a reference to the member?

lib/Sema/SemaInit.cpp
5381

Maybe pass the ExtendingEntity * into here instead of its decl and a reference to its mangling number?

5791–5808

You could reorder these to first create the MTE and then performReferenceExtension on it, and avoid the special-casing of the ManglingNumber here:

auto *MTE = new (S.Context) MaterializeTemporaryExpr(... /*no extending decl or mangling number*/ ...);
if (auto *ExtendingEntity = getEntityForTemporaryLifetimeExtension(&Entity)) {
  performReferenceExtension(MTE, ExtendingEntity);
  warnOnLifetimeExtension(...);
}
6196–6218

Likewise here.

majnemer updated this revision to Diff 9004.Apr 30 2014, 8:33 PM

Address Richard's review comments.

rsmith accepted this revision.May 1 2014, 1:45 AM
rsmith edited edge metadata.

Looks great, thanks!

include/clang/Sema/Initialization.h
424

Please give this a name that sounds more like it's changing the state of the object. allocateManglingNumber or something?

lib/AST/MicrosoftMangle.cpp
120

This line looks like it's over 80 characters to me.

This revision is now accepted and ready to land.May 1 2014, 1:45 AM
majnemer closed this revision.May 19 2014, 7:23 AM
majnemer updated this revision to Diff 9551.

Closed by commit rL207776 (authored by @majnemer).