This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Handle SubstNonTypeTemplateParmExprs
ClosedPublic

Authored by tbaeder on Aug 29 2022, 1:24 AM.

Details

Summary

These are also easy to add and used in a unit test that I'd like to get working with the new interpreter.

Diff Detail

Event Timeline

tbaeder created this revision.Aug 29 2022, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 1:24 AM
tbaeder requested review of this revision.Aug 29 2022, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 1:24 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
shafik accepted this revision.Aug 29 2022, 8:43 AM

Makes sense to me.

This revision is now accepted and ready to land.Aug 29 2022, 8:43 AM
erichkeane added inline comments.Aug 29 2022, 9:00 AM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
237

Is there nothing special that has to happen when these are reference parameters? Can you at least add a test for that?

tbaeder added inline comments.Aug 29 2022, 9:15 AM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
237

Umm, I don't really do references yet (I haven't tested them at all and I don't think they are implemented).

erichkeane added inline comments.Aug 29 2022, 9:43 AM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
237

Hmm... that is unfortunate. We really should be doing references ASAP, as they are going to be a pretty critical/oft-needed during testing kinda thing. I'd at least want a commented-out test that you can re-enable ASAP.

tahonermann added inline comments.Aug 29 2022, 1:23 PM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
237

Are not-yet-implemented features consistently implemented such that constant evaluation will fail with a (possibly unhelpful) diagnostic? As in https://godbolt.org/z/8vvdca9Ma? If so, can tests be added ahead of time with annotations for expected errors (essentially false positives to be addressed later)?

tbaeder added inline comments.Aug 29 2022, 9:48 PM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
237

Some are always (like unhandled AST node), some are not.

tbaeder marked 3 inline comments as done.Sep 1 2022, 12:39 AM
tbaeder added inline comments.
clang/lib/AST/Interp/ByteCodeExprGen.cpp
237

An example for this with a reference type is in https://reviews.llvm.org/D132997

aaron.ballman accepted this revision.Sep 2 2022, 12:21 PM

LGTM on the assumption that the extra test coverage doesn't find anything.

clang/test/AST/Interp/functions.cpp
68–69
70–75

Can you also add a use where there's a default argument for the NTTP just to make sure we get that right too? (I'd be surprised if it didn't.)

tbaeder updated this revision to Diff 457766.Sep 3 2022, 12:07 AM
tbaeder marked 2 inline comments as done.
erichkeane accepted this revision.Sep 6 2022, 6:07 AM
This revision was landed with ongoing or failed builds.Sep 7 2022, 10:32 PM
This revision was automatically updated to reflect the committed changes.