This is an archive of the discontinued LLVM Phabricator instance.

Differential D117301

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

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

Details

Reviewers
jfb
rsmith
RKSimon
nand
Commits
rG7cd441ff537e: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter
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.
Herald added a subscriber: cfe-commits. · View Herald TranscriptJan 14 2022, 4:33 AM
owenpan edited the summary of this revision. (Show Details)Jan 14 2022, 4:38 AM
Harbormaster completed remote builds in B143361: Diff 399953.Jan 14 2022, 5:07 AM
RKSimon added a comment.Jan 19 2022, 1:24 AM

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

owenpan added a comment.Jan 19 2022, 1:48 AM
In D117301#3253874, @RKSimon wrote:

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.
Harbormaster completed remote builds in B144315: Diff 401257.Jan 19 2022, 10:35 AM
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
Closed by commit rG7cd441ff537e: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter (authored by owenpan). · Explain Why
This revision was automatically updated to reflect the committed changes.
owenpan added a commit: rG7cd441ff537e: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter.
owenpan mentioned this in D120359: [clang-format][NFC] Remove redundant semi.Feb 23 2022, 12:36 AM

Revision Contents

PathSize
clang/
lib/
AST/
Interp/
50 lines

Diff 401257

clang/lib/AST/Interp/PrimType.h

Show First 20 Lines • Show All 74 Lines • ▼ Show 20 Linesinline bool isPrimitiveIntegral(PrimType Type) {
} }
} }
} // namespace interp } // namespace interp
} // namespace clang } // namespace clang
/// Helper macro to simplify type switches. /// Helper macro to simplify type switches.
/// The macro implicitly exposes a type T in the scope of the inner block. /// The macro implicitly exposes a type T in the scope of the inner block.
#define TYPE_SWITCH_CASE(Name, B) \ #define TYPE_SWITCH_CASE(Name, B) \
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-#define TYPE_SWITCH_CASE(Name, B) \
-  case Name: { using T = PrimConv<Name>::T; B; break; }
+#define TYPE_SWITCH_CASE(Name, B)                                              \
+  case Name: {                                                                 \
+    using T = PrimConv<Name>::T;                                               \
+    B;                                                                         \
+    break;                                                                     \
+  }
Lint: Pre-merge checks: clang-format: please reformat the code ``` -#define TYPE_SWITCH_CASE(Name, B) \ - case Name: {…
case Name: { using T = PrimConv<Name>::T; do {B;} while(0); break; } case Name: { using T = PrimConv<Name>::T; B; break; }
#define TYPE_SWITCH(Expr, B) \ #define TYPE_SWITCH(Expr, B) \
do { \
switch (Expr) { \ switch (Expr) { \
TYPE_SWITCH_CASE(PT_Sint8, B) \ TYPE_SWITCH_CASE(PT_Sint8, B) \
TYPE_SWITCH_CASE(PT_Uint8, B) \ TYPE_SWITCH_CASE(PT_Uint8, B) \
TYPE_SWITCH_CASE(PT_Sint16, B) \ TYPE_SWITCH_CASE(PT_Sint16, B) \
TYPE_SWITCH_CASE(PT_Uint16, B) \ TYPE_SWITCH_CASE(PT_Uint16, B) \
TYPE_SWITCH_CASE(PT_Sint32, B) \ TYPE_SWITCH_CASE(PT_Sint32, B) \
TYPE_SWITCH_CASE(PT_Uint32, B) \ TYPE_SWITCH_CASE(PT_Uint32, B) \
TYPE_SWITCH_CASE(PT_Sint64, B) \ TYPE_SWITCH_CASE(PT_Sint64, B) \
TYPE_SWITCH_CASE(PT_Uint64, B) \ TYPE_SWITCH_CASE(PT_Uint64, B) \
TYPE_SWITCH_CASE(PT_Bool, B) \ TYPE_SWITCH_CASE(PT_Bool, B) \
TYPE_SWITCH_CASE(PT_Ptr, B) \ TYPE_SWITCH_CASE(PT_Ptr, B) \
} } \
} while (0)
#define COMPOSITE_TYPE_SWITCH(Expr, B, D) \ #define COMPOSITE_TYPE_SWITCH(Expr, B, D) \
do { \
switch (Expr) { \ switch (Expr) { \
TYPE_SWITCH_CASE(PT_Ptr, B) \ TYPE_SWITCH_CASE(PT_Ptr, B) \
default: do { D; } while(0); break; \ default: { D; break; } \
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-      default: { D; break; }                                                   \
+    default: {                                                                 \
+      D;                                                                       \
+      break;                                                                   \
+    }                                                                          \
Lint: Pre-merge checks: clang-format: please reformat the code ``` - default: { D; break; }…
} } \
#define INT_TYPE_SWITCH(Expr, B) \ } while (0)
switch (Expr) { \
TYPE_SWITCH_CASE(PT_Sint8, B) \
TYPE_SWITCH_CASE(PT_Uint8, B) \
TYPE_SWITCH_CASE(PT_Sint16, B) \
TYPE_SWITCH_CASE(PT_Uint16, B) \
TYPE_SWITCH_CASE(PT_Sint32, B) \
TYPE_SWITCH_CASE(PT_Uint32, B) \
TYPE_SWITCH_CASE(PT_Sint64, B) \
TYPE_SWITCH_CASE(PT_Uint64, B) \
default: llvm_unreachable("not an integer"); \
}
#endif #endif