[libcxx-commits] [PATCH] D94530: [libc++] improve feature test macro script

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 12 10:28:36 PST 2021

Quuxplusone added a comment.

Seems like a generally good direction to me, FWIW.

Comment at: libcxx/utils/generate_feature_test_macro_components.py:903
+  std_tests.append(f'#if TEST_STD_VER < {get_std_nr(std_dialects[1])}\n\n' +
+                   generate_std_test(test_list, std_dialects[0]))
I see no obvious reason for the special case here. Sure we //currently// do it as `#if TEST_STD_VER < 14`, but would anything really break if we did it as `#if TEST_STD_VER == 11` instead? It'd be worth trying to find out.

Comment at: libcxx/utils/generate_feature_test_macro_components.py:916
+  else:
+    if_condition = '> ' + get_std_nr(std_dialects[-2])
I think this should just be `assert not std_dialects[-1][-1].isnumeric()`, and then simplify accordingly. (That would technologically enforce my earlier opinion that whoever changes `2a` to `20` should go ahead and add `2b` at the same time.)

Comment at: libcxx/utils/generate_feature_test_macro_components.py:923
+  return '\n\n'.join(std_tests)
FWIW, I would recommend copying the style from circa line 800: use `template.format("""...""")` instead of f-strings, and use one big `"""` string with all the arguments collected at the bottom, instead of a bunch of little strings `append`'ed together. (This also solves your question about whether f-strings are allowed, because you'd no longer need to use f-strings.)

Comment at: libcxx/utils/generate_feature_test_macro_components.py:1027
   table = [["Macro Name", "Value"]]
-  for std in get_std_dialects():
+  for std in get_std_dialects()[1:]:
     table += [["**" + std.replace("c++", "C++ ") + "**", ""]]
This `[1:]` smells questionable.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list