This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Check Neg ops for errors
ClosedPublic

Authored by tbaeder on Apr 22 2023, 5:11 AM.

Details

Summary

I'm not sure what to do with the float case here, maybe it's not relevant at all.

Diff Detail

Event Timeline

tbaeder created this revision.Apr 22 2023, 5:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2023, 5:11 AM
tbaeder requested review of this revision.Apr 22 2023, 5:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2023, 5:11 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aaron.ballman added inline comments.Apr 25 2023, 9:35 AM
clang/lib/AST/Interp/Interp.h
438–441

I'm confused -- why would negation be UB for a floating-point type? For integer types, it's a matter of not being able to represent the value when the source is INT_MIN.

tbaeder added inline comments.Apr 25 2023, 10:25 AM
clang/lib/AST/Interp/Interp.h
438–441

No idea, this was just something that came to mind when I wrote the code. If it's not an issue, that's even better can I can just remove the comment.

aaron.ballman added inline comments.Apr 25 2023, 12:15 PM
clang/lib/AST/Interp/Interp.h
438–441

I think we should probably assert that the failure only occurs for integer types and not other arithmetic types.

tbaeder updated this revision to Diff 517054.Apr 25 2023, 9:56 PM
tbaeder marked an inline comment as done.
aaron.ballman accepted this revision.Apr 26 2023, 4:39 AM

LGTM with a request to add a message to the assertion.

clang/lib/AST/Interp/Interp.h
441
This revision is now accepted and ready to land.Apr 26 2023, 4:39 AM
This revision was landed with ongoing or failed builds.Apr 27 2023, 3:05 AM
This revision was automatically updated to reflect the committed changes.