aaron.ballman added a comment.
The summary for the patch explains what's being added, but there's really no
information as to why it's being added. Why do we need these builtins?
================
Comment at: clang/include/clang/AST/Type.h:743
+ bool isReferenceable() const;
+
----------------
Doc comments would be handy here because this is a pretty recent term of art.
================
Comment at: clang/include/clang/AST/Type.h:6480
+ // type that is either an object type, a function type that does not have
+ // cv-qualifiers or a ref-qualifier, or a reference type
+ const Type &Self = **this;
----------------
================
Comment at: clang/include/clang/AST/Type.h:6481
+ // cv-qualifiers or a ref-qualifier, or a reference type
+ const Type &Self = **this;
+ if (Self.isObjectType() || Self.isReferenceType())
----------------
This is unsafe -- the `Type *` can be null if the `QualType` is invalid.
================
Comment at: clang/include/clang/AST/Type.h:6488
+ const auto *F = Self.getAs<FunctionProtoType>();
+ return F == nullptr ||
+ (F->getMethodQuals().empty() && F->getRefQualifier() == RQ_None);
----------------
A function without a prototype is referenceable? (This is more of a "should
this predicate do anything in C?" kind of question, I suppose.)
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8595
+def err_make_signed_integral_only : Error<
+ "'%select{make_unsigned|make_signed}0' is only compatible with non-bool
integers and enum types, but was given %1">;
def ext_typecheck_cond_incompatible_pointers : ExtWarn<
----------------
This can be reformatted, I believe, but did you look around to see if an
existing diagnostic would suffice?
================
Comment at: clang/lib/Sema/DeclSpec.cpp:597
case DeclSpec::TST_decltype_auto: return "decltype(auto)";
+ // clang-format off
case DeclSpec::TST_underlyingType: return "__underlying_type";
----------------
We don't typically add clang-format markings to the source files. I think this
should be removed (it disables the formatting for the remainder of the file).
================
Comment at: clang/lib/Sema/SemaType.cpp:1265
+TSTToUnaryTransformType(DeclSpec::TST SwitchTST) {
+ switch (SwitchTST) { // clang-format off
+ case TST_add_const: return UnaryTransformType::AddConst;
----------------
Same comments here about the clang-format markup.
================
Comment at: clang/lib/Sema/SemaType.cpp:1694
if (Result.isNull()) {
- Result = Context.IntTy;
+ if (DS.getTypeSpecType() == DeclSpec::TST_underlyingType)
+ Result = Context.IntTy;
----------------
The point to this was for error recovery; can we recover enough type
information from the non-underlying type cases to recover similarly?
================
Comment at: clang/lib/Sema/SemaType.cpp:6023
void VisitUnaryTransformTypeLoc(UnaryTransformTypeLoc TL) {
- // FIXME: This holds only because we only have one unary transform.
- assert(DS.getTypeSpecType() == DeclSpec::TST_underlyingType);
+ // Make sure it is a unary transform type
+ assert(DS.getTypeSpecType() >= DeclSpec::TST_underlyingType &&
----------------
================
Comment at: clang/lib/Sema/SemaType.cpp:9121
+ SourceLocation Loc) {
+ if (!BaseType->isPointerType())
+ return Context.getUnaryTransformType(BaseType, BaseType, UKind);
----------------
Should we care about ObjC pointers (which are a bit special)?
================
Comment at: clang/lib/Sema/SemaType.cpp:9136
+ Qualifiers Quals = Underlying.getQualifiers();
+ Quals.removeCVRQualifiers();
+ return Context.getUnaryTransformType(
----------------
What about things like type attributes (are those lost during decay)?
================
Comment at: clang/test/SemaCXX/type-traits.cpp:3447
+ { int a[T(__is_same(add_cv_t<M>, const volatile M))]; }
+ { int a[T(__is_same(add_lvalue_reference_t<M>, M))]; }
+ { int a[T(__is_same(add_pointer_t<M>, M))]; }
----------------
Shouldn't this be the same as `M&`?
Actually, something funky is going on that I've not looked into very far. When
I try the test out and use `M&` instead of `M` here:
https://godbolt.org/z/rx5bcsfqe, so we should be sure we're instantiating the
tests we expect to instantiate.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116203/new/
https://reviews.llvm.org/D116203
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
