[libcxx-commits] [PATCH] D99801: [libcxx] adds Microsoft/STL tests for P0898R3

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I really like that we use these Microsoft tests!
I would like to give some thought how we'll do this since I like to use this as a precedent. D70631 <https://reviews.llvm.org/D70631> also contains Microsoft tests, but uses a different directory and a different approach to including the tests. (This patch will be done before D70631 <https://reviews.llvm.org/D70631> so best to use this one to discuss it.)



================
Comment at: libcxx/test/msvc-stl/P0898R3_concepts/invocable_cc.hpp:1
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
----------------
I like the tests in this separate directory.


================
Comment at: libcxx/test/msvc-stl/README.md:1
+# Alterations
+
----------------
Please add some information about how to add Microsoft unit tests to libc++ in `docs/Contributing.rst`, which then refers to this document for the details.


================
Comment at: libcxx/test/msvc-stl/README.md:9
+* `test.cpp` files have been renamed to `test.pass.cpp`.
+* LIT directives have been applied to all `test.pass.cpp` files.
+* A single `// clang-format off` has been added to each file underneath LIT directives.
----------------
What I did in D70631 was a slightly different approach. I kept `test.cpp` and added `test.pass.cpp` The new file consists of:
```
// Copyright block

// Lit directives

#include "test.cpp"
```
This change hasn't be reviewed so I don't know whether it's acceptable.
I choose this approach to the Microsoft files don't contain libc++ lit directives and there would be no need to rename the file.

WDYT of this approach?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99801/new/

https://reviews.llvm.org/D99801