This is an archive of the discontinued LLVM Phabricator instance.

[clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter
ClosedPublic

Authored by owenpan on Jan 14 2022, 4:33 AM.

Details

Summary

Wraps the expansion of TYPE_SWITCH of the constexpr interpreter
in do { ... } while (0) so that the macro can be used like this:

if (llvm::Optional<PrimType> T = Ctx.classify(FieldTy)) 
  TYPE_SWITCH(*T, Ok &= ReturnValue<T>(FP.deref<T>(), Value));
else 
  Ok &= Composite(FieldTy, FP, Value);

This bug was found while testing D116316. See also this review comment.

Diff Detail

Event Timeline

owenpan requested review of this revision.Jan 14 2022, 4:33 AM
owenpan created this revision.
owenpan edited the summary of this revision. (Show Details)Jan 14 2022, 4:38 AM

what about COMPOSITE_TYPE_SWITCH and INT_TYPE_SWITCH - should they be updated as well?

what about COMPOSITE_TYPE_SWITCH and INT_TYPE_SWITCH - should they be updated as well?

I left them alone because COMPOSITE_TYPE_SWITCH didn't cause a dangling else problem and INT_TYPE_SWITCH was not used at all. I can fix them both or fix the former and remvoe the latter from the header file. Also, the current do { ... } while (0) statements in TYPE_SWITCH_CASE and COMPOSITE_TYPE_SWITCH are superfluous. I can remove them as well.

owenpan updated this revision to Diff 401257.Jan 19 2022, 8:40 AM
  • Also wrapped COMPOSITE_TYPE_SWITCH with do-while.
  • Cleaned up the macro definitions by removing the superfluous do-while statements.
  • Removed the unused INT_TPYE_SWITCH macro.
RKSimon accepted this revision.Jan 24 2022, 4:03 AM

LGTM - cheers

This revision is now accepted and ready to land.Jan 24 2022, 4:03 AM
This revision was landed with ongoing or failed builds.Jan 24 2022, 9:05 AM
This revision was automatically updated to reflect the committed changes.