[clang-tools-extra] [libc] Fix printf config not working (PR #66834)

Siva Chandra via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 20 11:10:10 PDT 2023


================
@@ -112,6 +112,16 @@ add_libc_unittest(
     LibcMemoryHelpers
 )
 
+if(LIBC_CONF_PRINTF_DISABLE_FLOAT)
----------------
sivachandra wrote:

I do not think this pattern is how the copts are to be used. In this particular case, I think the right way would be to split `sprintf_test` into `sprintf_basic_test`, `sprintf_float_test`, `sprintf_index_mode_test`, `sprintf_write_int_test` etc. Then, test targets should be added as follows:

```
add_libc_test(
  sprintf_basic_test
  ...
)

if(NOT LIBC_CONF_PRINTF_DISABLE_FLOAT)
  add_fp_unittest(
    sprintf_float_test
  )
endif()

# Other conditionals on config options
```

There could be some compile opts that need to be inherited by the tests [1]. Even in such a case, the test source code should not directly use those internal copts - they are internal implementation details unrelated to the tests.

[1] - We do not have such a mechanism set up currently. We can add it when required. It will be required if the tests are calling internal implementation functions and not the entrypoints. We do have such tests but they are typically testing common infrastructure components not affected by config options.

https://github.com/llvm/llvm-project/pull/66834


More information about the cfe-commits mailing list