)]}' { "commit": "32ef0ecca5d8cc875453541da5be5443839e9768", "tree": "dd90a64af29cd3596a2a08fb98e69a4835801ed2", "parents": [ "3b64e7030f8e8e0fd94c7fbd80edb22ac9856276" ], "author": { "name": "Pedro Alves", "email": "pedro@palves.net", "time": "Fri Aug 21 15:36:37 2020 +0100" }, "committer": { "name": "Pedro Alves", "email": "pedro@palves.net", "time": "Fri Aug 21 15:42:32 2020 +0100" }, "message": "Rewrite enum_flags, add unit tests, fix problems\n\nThis patch started by adding comprehensive unit tests for enum_flags.\n\nFor the testing part, it adds:\n\n - tests of normal expected uses of the API.\n\n - checks that _invalid_ uses of the API would fail to compile. I.e.,\n it validates that enum_flags really is a strong type, and that\n incorrect mixing of enum types would be caught at compile time. It\n pulls that off making use of SFINEA and C++11\u0027s decltype/constexpr.\n\nThis revealed many holes in the enum_flags API. For example, the f1\nassignment below currently incorrectly fails to compile:\n\n enum_flags\u003cflags\u003e f1 \u003d FLAG1;\n enum_flags\u003cflags\u003e f2 \u003d FLAG2 | f1;\n\nThe unit tests also revealed that this useful use case doesn\u0027t work:\n\n enum flag { FLAG1 \u003d 1, FLAG2 \u003d 2 };\n enum_flags\u003cflag\u003e src \u003d FLAG1;\n enum_flags\u003cflag\u003e f1 \u003d condition ? src : FLAG2;\n\nIt fails to compile because enum_flags\u003cflag\u003e and flag are convertible\nto each other.\n\nTurns out that making enum_flags be implicitly convertible to the\nbacking raw enum type was not a good idea.\n\nIf we make it convertible to the underlying type instead, we fix that\nternary operator use case, and, we find cases throughout the codebase\nthat should be using the enum_flags but were using the raw backing\nenum instead. So it\u0027s a good change overall.\n\nAlso, several operators were missing.\n\nThese holes and more are plugged by this patch, by reworking how the\nenum_flags operators are implemented, and making use of C++11\u0027s\nfeature of being able to delete methods/functions.\n\nThere are cases in gdb/compile/ where we need to call a function in a\nC plugin API that expects the raw enum. To address cases like that,\nthis adds a \"raw()\" method to enum_flags. This way we can keep using\nthe safer enum_flags to construct the value, and then be explicit when\nwe need to get at the raw enum.\n\nThis makes most of the enum_flags operators constexpr. Beyond\nenabling more compiler optimizations and enabling the new unit tests,\nthis has other advantages, like making it possible to use operator|\nwith enum_flags values in switch cases, where only compile-time\nconstants are allowed:\n\n enum_flags\u003cflags\u003e f \u003d FLAG1 | FLAG2;\n switch (f)\n {\n case FLAG1 | FLAG2:\n\tbreak;\n }\n\nCurrently that fails to compile.\n\nIt also switches to a different mechanism of enabling the global\noperators. The current mechanism isn\u0027t namespace friendly, the new\none is.\n\nIt also switches to C++11-style SFINAE -- instead of wrapping the\nreturn type in a SFINAE-friently structure, we use an unnamed template\nparameter. I.e., this:\n\n template \u003ctypename enum_type,\n\t typename \u003d is_enum_flags_enum_type_t\u003cenum_type\u003e\u003e\n enum_type\n operator\u0026 (enum_type e1, enum_type e2)\n\ninstead of:\n\n template \u003ctypename enum_type\u003e\n typename enum_flags_type\u003cenum_type\u003e::type\n operator\u0026 (enum_type e1, enum_type e2)\n\nNote that the static_assert inside operator~() was converted to a\ncouple overloads (signed vs unsigned), because static_assert is too\nlate for SFINAE-based tests, which is important for the CHECK_VALID\nunit tests.\n\nTested with gcc {4.8, 7.1, 9.3} and clang {5.0.2, 10.0.0}.\n\ngdb/ChangeLog:\n\n\t* Makefile.in (SELFTESTS_SRCS): Add\n\tunittests/enum-flags-selftests.c.\n\t* btrace.c (ftrace_update_caller, ftrace_fixup_calle): Use\n\tbtrace_function_flags instead of enum btrace_function_flag.\n\t* compile/compile-c-types.c (convert_qualified): Use\n\tenum_flags::raw.\n\t* compile/compile-cplus-symbols.c (convert_one_symbol)\n\t(convert_symbol_bmsym):\n\t* compile/compile-cplus-types.c (compile_cplus_convert_method)\n\t(compile_cplus_convert_struct_or_union_methods)\n\t(compile_cplus_instance::convert_qualified_base):\n\t* go-exp.y (parse_string_or_char): Add cast to int.\n\t* unittests/enum-flags-selftests.c: New file.\n\t* record-btrace.c (btrace_thread_flag_to_str): Change parameter\u0027s\n\ttype to btrace_thread_flags from btrace_thread_flag.\n\t(record_btrace_cancel_resume, record_btrace_step_thread): Change\n\tlocal\u0027s type to btrace_thread_flags from btrace_thread_flag. Add\n\tcast in DEBUG call.\n\ngdbsupport/ChangeLog:\n\n\t* common/enum-flags.h: Include \"traits.h\".\n\t(DEF_ENUM_FLAGS_TYPE): Declare a function instead of defining a\n\tstructure.\n\t(enum_underlying_type): Update comment.\n\t(namespace enum_flags_detail): New. Move struct zero_type here.\n\t(EnumIsUnsigned, EnumIsSigned): New.\n\t(class enum_flags): Make most methods constexpr.\n\t(operator\u0026\u003d, operator|\u003d, operator^\u003d): Take an enum_flags instead\n\tof an enum_type.\n\t(operator enum_type()): Delete.\n\t(operator\u0026, operator|, operator^, operator~): Delete, moved out of\n\tclass.\n\t(raw()): New method.\n\t(is_enum_flags_enum_type_t): Declare.\n\t(ENUM_FLAGS_GEN_BINOP, ENUM_FLAGS_GEN_COMPOUND_ASSIGN)\n\t(ENUM_FLAGS_GEN_COMP): New. Use them to reimplement global\n\toperators.\n\t(operator~): Now constexpr and reimplemented.\n\t(operator\u003c\u003c, operator\u003e\u003e): New deleted functions.\n\t* valid-expr.h (CHECK_VALID_EXPR_5, CHECK_VALID_EXPR_6): New.", "tree_diff": [ { "type": "modify", "old_id": "66abac4b15a488cc45a88a451340fd90f3fddf0b", "old_mode": 33188, "old_path": "gdb/Makefile.in", "new_id": "a683d232da2c7687c79c4d3cacf790f55f3ab1bd", "new_mode": 33188, "new_path": "gdb/Makefile.in" }, { "type": "modify", "old_id": "2a0c61de766a53ecb370149f9b6c340a7e86d2d3", "old_mode": 33188, "old_path": "gdb/btrace.c", "new_id": "9022aedd1c804bd71ba2ce84a76348098b10e53e", "new_mode": 33188, "new_path": "gdb/btrace.c" }, { "type": "modify", "old_id": "2b25783bb00ca0c9e693bd5168cf162d9ba1ec19", "old_mode": 33188, "old_path": "gdb/compile/compile-c-types.c", "new_id": "0234db59ea97490877104e289dead1b2290f24e0", "new_mode": 33188, "new_path": "gdb/compile/compile-c-types.c" }, { "type": "modify", "old_id": "11a2d3234586f542ba259ae60157a93514345a25", "old_mode": 33188, "old_path": "gdb/compile/compile-cplus-symbols.c", "new_id": "9840485039a2da9831aeaf870d9c08dfb3508efb", "new_mode": 33188, "new_path": "gdb/compile/compile-cplus-symbols.c" }, { "type": "modify", "old_id": "02df7ab90e6e4103103c8c9807b7a691393532e4", "old_mode": 33188, "old_path": "gdb/compile/compile-cplus-types.c", "new_id": "022cc88979444813355d3dba17c7d072e8070190", "new_mode": 33188, "new_path": "gdb/compile/compile-cplus-types.c" }, { "type": "modify", "old_id": "17c76ac02ab07dc2baa073cb00304af9b8c061e1", "old_mode": 33188, "old_path": "gdb/go-exp.y", "new_id": "ee1db2b5874220e5405e5013db1a5666624482c5", "new_mode": 33188, "new_path": "gdb/go-exp.y" }, { "type": "modify", "old_id": "a1a3efc3d681404c5ed2f7edd9f95e379e291915", "old_mode": 33188, "old_path": "gdb/record-btrace.c", "new_id": "fd0d13fb25892c33721037eb293f97dded3a98fa", "new_mode": 33188, "new_path": "gdb/record-btrace.c" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "17ab5c9b094927b9ba112996afed696a424bb51a", "new_mode": 33188, "new_path": "gdb/unittests/enum-flags-selftests.c" }, { "type": "modify", "old_id": "825ff4faf2c41e86da12a71ade5b2898c1b4f098", "old_mode": 33188, "old_path": "gdbsupport/enum-flags.h", "new_id": "b3e317ecb973de844735b97865d4dcebfb5b633f", "new_mode": 33188, "new_path": "gdbsupport/enum-flags.h" }, { "type": "modify", "old_id": "a22fa61134f0c497c525a27db69178fb040d6c52", "old_mode": 33188, "old_path": "gdbsupport/valid-expr.h", "new_id": "459de179266a50c31ca149d4b9c0c68b61cc40d0", "new_mode": 33188, "new_path": "gdbsupport/valid-expr.h" } ] }