[PATCH] D31739: Add markup for libc++ dylib availability

Jonathan Roelofs via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 16 08:17:13 PDT 2017


jroelofs added inline comments.


================
Comment at: utils/libcxx/test/config.py:289
+    def configure_availability(self):
+        # FIXME doc
+        self.with_availability = self.get_lit_bool('with_availability', False)
----------------
Can you expand on what the FIXME here wants? Is there something that AvailabilityMarkup.rst doesn't cover?


================
Comment at: utils/libcxx/test/config.py:316
+        if self.with_availability:
+            self.use_clang_verify = False
+            return
----------------
Why does the availability stuff clash with -verify?


================
Comment at: utils/libcxx/test/config.py:363
+        self.config.available_features.add('%s=%s%s' % (feature, name, version))
+        self.config.available_features.add('%s=%s%s' % (feature, name, version))
+
----------------
This line, and the one above it are the same. Is that intentional?


================
Comment at: utils/libcxx/test/config.py:387
+                self.config.available_features.add(
+                    'with_system_cxx_lib=%s' % component)
 
----------------
Is it worth filtering out `none` and `unknown`, as they're often repeated, and you can't tell which part of the triple they came from?

Consider: `arm-none-linux-gnueabi` vs `arm-none-none-eabi`.

Or would it be better to include some marker in the features to say which part of the triple it came from, eg:

```
  - with_system_cxx_lib=arch:arm
  - with_system_cxx_lib=vendor:none
  - with_system_cxx_lib=os:linux
  - with_system_cxx_lib=sys:gnueabi
```


https://reviews.llvm.org/D31739





More information about the cfe-commits mailing list