[PATCH] D135088: [Clang] make canonical AutoType constraints-free

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:5730-5732
   if (TypeConstraintConcept)
     TypeConstraintConcept = TypeConstraintConcept->getCanonicalDecl();
 
----------------
Pre-existing problem: This was not good to begin with, since we would not capture the correct declaration used, but now you can just remove it, no further changes required.


================
Comment at: clang/lib/AST/ASTContext.cpp:5736-5737
   llvm::FoldingSetNodeID ID;
   AutoType::Profile(ID, *this, DeducedType, Keyword, IsDependent,
                     TypeConstraintConcept, TypeConstraintArgs);
   if (AutoType *AT = AutoTypes.FindNodeOrInsertPos(ID, InsertPos))
----------------
FYI (not asking you to fix this on this patch) there is a pre-existing bug here where we are not profiling the node on the `isPack` flag.


================
Comment at: clang/lib/AST/ASTContext.cpp:5745-5746
+      if (TypeConstraintConcept) {
+        Canon = getAutoTypeInternal(QualType(), Keyword, false, IsPack, nullptr,
+                                    {}, true);
         // Find the insert position again.
----------------
Also adds back the `isDependent` flag. Any reason to have removed it?


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8664-8668
+  if (QualType RT = FD->getDeclaredReturnType();
+      DCK == DefaultedComparisonKind::ThreeWay &&
+      RT->getContainedDeducedType() &&
+      (!Context.hasSameType(RT, Context.getAutoDeductType()) ||
+       RT->getContainedAutoType()->isConstrained())) {
----------------
I think this might crash if the three-way comparison is defaulted with a deduced template specialization.

Something like:
```
template <class> struct A {};

struct P {
  friend A operator<=>(const P&, const P&) = default;
};
```

The problem is that `RT->getContainedDeducedType()` will return non-null, because DTSTs are also deduced types, but `RT->getContainedAutoType()` will return null, because DTSTs are not AutoTypes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135088