Page MenuHomePhabricator

[Sema] Prevent binding references with mismatching address spaces to temporaries
ClosedPublic

Authored by Anastasia on Apr 30 2019, 9:12 AM.

Details

Summary

This is a straw-man idea for solving binding references with address spaces to temporaries. If the logic below makes sense I will apply it elsewhere in similar checks.

Arbitrary address space references can't be bound to temporaries. The reference binding logic should check that the address space of temporary can be implicitly converted to address space in reference when temporary materialization is performed.

Example:

_AS1 float gf;
...
_AS1 const int& ref = gf; //this can only be accepted if implicit conversion from _AS1 to LangAS::Default is allowed

This review depends on: https://reviews.llvm.org/D58060

Diff Detail

Repository
rL LLVM

Event Timeline

Anastasia created this revision.Apr 30 2019, 9:12 AM
rjmccall added inline comments.Apr 30 2019, 4:17 PM
lib/Sema/SemaInit.cpp
4836 ↗(On Diff #197349)

Isn't cv1T1IgnoreAS.getQualifiers() always LangAS::Default?

Anastasia marked an inline comment as done.May 1 2019, 2:03 AM
Anastasia added inline comments.
lib/Sema/SemaInit.cpp
4836 ↗(On Diff #197349)

Yes, this is true currently. However, we don't have an overload of isAddressSpaceSupersetOf that takes LangAS as a parameter. Are you suggesting to add it?

rjmccall added inline comments.May 1 2019, 10:17 AM
lib/Sema/SemaInit.cpp
4836 ↗(On Diff #197349)

I think it would be sensible if there was a function that could answer the superset question with just two LangAS values, yeah. Even with the current API, it would at least be clearer if you just manufactured a Qualifiers with no address space, since there's no dependency on any of the other qualifiers from cv1T1IgnoreAS.

Anastasia updated this revision to Diff 197769.May 2 2019, 6:42 AM

Added extra overload for isAddressSpaceSupersetOf.

rjmccall added inline comments.May 2 2019, 11:29 AM
include/clang/AST/Type.h
463 ↗(On Diff #197769)

"equal to or a superset of", just for clarity's sake.

478 ↗(On Diff #197769)

How about: "if the address space in these qualifiers is equal to or a superset of the address space in the argument qualifiers".

Anastasia updated this revision to Diff 197940.May 3 2019, 3:02 AM

Updated comments!

rjmccall accepted this revision.May 3 2019, 9:24 AM

LGTM.

This revision is now accepted and ready to land.May 3 2019, 9:24 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jun 5, 7:00 AM