This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Reject reinterpret_cast expressions
ClosedPublic

Authored by tbaeder on Jun 19 2023, 7:27 AM.

Details

Summary

Add a new InvalidCast op for this purpose and emit a diagnostic.

Diff Detail

Event Timeline

tbaeder created this revision.Jun 19 2023, 7:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2023, 7:27 AM
tbaeder requested review of this revision.Jun 19 2023, 7:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2023, 7:27 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aaron.ballman added inline comments.Jun 21 2023, 6:08 AM
clang/lib/AST/Interp/Interp.h
1761

Should this be doing the cast? The overloaded operator accepts an interp::CastKind already?

clang/lib/AST/Interp/PrimType.h
48

Should we bother adding this right now given it's not used or tested?

tbaeder added inline comments.Jun 21 2023, 6:12 AM
clang/lib/AST/Interp/Interp.h
1761

The streaming operator there is for our dump() support, and the note_constexpr_invalid_cast diagnostic needs an integer to select the right string.

clang/lib/AST/Interp/PrimType.h
48

No strong opinion from my side, I can definitely remove it.

aaron.ballman accepted this revision.Jun 21 2023, 6:15 AM

LGTM with the Dynamic bit pulled.

clang/lib/AST/Interp/Interp.h
1761

Ah, that's right, this isn't calling the diagnostic streaming operator. Nevermind. :-)

clang/lib/AST/Interp/PrimType.h
48

Let's yank it for the moment.

This revision is now accepted and ready to land.Jun 21 2023, 6:15 AM
tbaeder updated this revision to Diff 533238.Jun 21 2023, 6:24 AM
This revision was landed with ongoing or failed builds.Jul 26 2023, 12:59 AM
This revision was automatically updated to reflect the committed changes.
jplehr added a subscriber: jplehr.Jul 26 2023, 1:16 AM

Hi,
this seems to have broken the OpenMP AMDGPU buildbot (https://lab.llvm.org/buildbot/#/builders/193/builds/35471)
I'm happy to help if needed.

jplehr added a comment.EditedJul 26 2023, 1:19 AM

Wow, thanks for the quick fix!
[edit]: Buildbot green again [/edit]

probinson added inline comments.
clang/lib/AST/Interp/Interp.h
1761

Would you mind changing this cast from uint8_t to unsigned? We have an internal bot using a pickier mode of MSVC and it complains about ambiguous overloads, as the operator<< doesn't have anything smaller than int and unsigned.

tbaeder added inline comments.Aug 2 2023, 6:51 AM
clang/lib/AST/Interp/Interp.h
1761

Feel free to push that change, I'm currently working on something else.

aaron.ballman added inline comments.Aug 2 2023, 6:52 AM
clang/lib/AST/Interp/Interp.h
1761

Yeah, that's a perfectly reasonable NFC commit to make -- go for it!