[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.


================
Comment at: clang/lib/Parse/ParseExpr.cpp:1750
+#undef TRANSFORM_TYPE_TRAIT_DEF
+    if (NextToken().is(tok::less)) {
+      Tok.setKind(tok::identifier);
----------------
rsmith wrote:
> Here you're checking for `trait<` and treating it as an identifier; elsewhere you're checking for `trait(` and treating it as a builtin. Is there a reason to treat `trait` followed by a token other than `(` or `<` inconsistently?
The parser needs to treat `trait<` as an identifier to accommodate libstdc++'s usage of a few of these as alias templates. I've added a comment to explain why in the code.


================
Comment at: clang/lib/Sema/SemaType.cpp:9251
+      BaseType->isBooleanType()) {
+    Diag(Loc, diag::err_make_signed_integral_only) << IsMakeSigned << BaseType;
+    return QualType();
----------------
cjdb wrote:
> rsmith wrote:
> > Should we support vector types here?
> Is it a conforming extension for `make_signed_t<int __attribute__((ext_vector_type(2)))>` to work?
Gentle ping.


================
Comment at: clang/test/SemaCXX/type-traits.cpp:3099-3100
+
+  { int a[T(__is_same(remove_pointer_t<int __attribute__((address_space(1))) *>, int))]; }
+  { int a[T(__is_same(remove_pointer_t<S __attribute__((address_space(2))) *>, S))]; }
+}
----------------
cjdb wrote:
> rsmith wrote:
> > It seems strange to remove the address space qualifier here, given that this trait doesn't remove cv-qualifiers.
> Does `int __attribute__((address_space(1)))` make sense as a type? I thought it had to be a pointee.
Gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203