This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Handle DeclRefExpr of reference types
ClosedPublic

Authored by tbaeder on Aug 31 2022, 12:29 AM.

Details

Summary

And add a couple of tests for it.

Notably, this is missing MaterializeTemporaryExpr and ExprWithCleanups, but the basic support seems to work.

Diff Detail

Event Timeline

tbaeder created this revision.Aug 31 2022, 12:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 12:29 AM
tbaeder requested review of this revision.Aug 31 2022, 12:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 12:29 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tbaeder updated this revision to Diff 456917.Aug 31 2022, 3:36 AM

I think like @aaron.ballman was saying in another PR you should aim for as complete set of tests as possible even if you have to comment one that can't work yet. For example cases that would involve MemberExpr or UsingShadow for example as well as the cases you mentioned not implemented in your description.

tbaeder updated this revision to Diff 457193.Sep 1 2022, 1:15 AM

I think like @aaron.ballman was saying in another PR you should aim for as complete set of tests as possible even if you have to comment one that can't work yet. For example cases that would involve MemberExpr or UsingShadow for example as well as the cases you mentioned not implemented in your description.

I get it for the MaterializeTemporaryExprs, but I don't understand why I would add tests using MemberExpr now and not when I work on record types (and references have already landed).

tbaeder updated this revision to Diff 457498.Sep 1 2022, 9:55 PM

I think like @aaron.ballman was saying in another PR you should aim for as complete set of tests as possible even if you have to comment one that can't work yet. For example cases that would involve MemberExpr or UsingShadow for example as well as the cases you mentioned not implemented in your description.

I get it for the MaterializeTemporaryExprs, but I don't understand why I would add tests using MemberExpr now and not when I work on record types (and references have already landed).

The test case I was thinking of for that is:

struct S {
  int i, j;
};

constexpr int test() {
  S s{1, 2};

  int &j = s.i;
  j += 10;

  return j;
}

static_assert(test() == 11, "");

where the reference is a DeclRefExpr. Even if structures don't work yet, capturing it as a test case now means we don't have to remember to think about "what if there's a reference to this member" when structures are implemented.

clang/test/AST/Interp/references.cpp
14
tbaeder updated this revision to Diff 458662.Sep 7 2022, 11:56 PM
tbaeder marked an inline comment as done.
aaron.ballman accepted this revision.Sep 8 2022, 10:38 AM

LGTM aside from a request for a comment in the test, thanks!

clang/test/AST/Interp/references.cpp
91
This revision is now accepted and ready to land.Sep 8 2022, 10:38 AM
This revision was automatically updated to reflect the committed changes.