[clang] NFC: refactor multiple implementations of getDecltypeForParenthesizedExpr

[mizvekov]

Summary

This cleanup patch refactors a bunch of functional duplicates of
getDecltypeForParenthesizedExpr into a common implementation.

Signed-off-by: Matheus Izvekov <[e-mail]>

[mizvekov]

.

[aaronpuchert]

It seems that using is*Value and the introduction of getDecltypeForParenthesizedExpr could be two separate changes.

clang/lib/Sema/SemaExprCXX.cpp

5895

The quote doesn't reference parenthesized expressions, isn't this just coincidentally the same thing?

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

76

This seems also more of a coincidence. There is no parenthesized expression, we're just trying to figure out a function return type.

(Ok, it's not a pure coincidence, the decltype is probably chosen to match that type.)

[mizvekov]

clang/lib/Sema/SemaExprCXX.cpp

5895

It's fundamentally the same thing. The getDecltypeForParenthesizedExpr name is what I tried to keep, I don't have better ideas there.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

76

Yes, not a coincidence, still fundamentally the same thing.

[mizvekov]

Split 'getValueKind' cleanup into a separate patch.

[aaronpuchert]

Again, I'm not sure if it helps to use getDecltypeForParenthesizedExpr where we don't actually have the decltype of a parenthesized expression.

It's probably not entirely coincidental, but things aren't defined this way.

clang/lib/Sema/SemaExprCXX.cpp

5895

What this is doing is pointwise equal to getDecltypeForParenthesizedExpr, but there is no parenthesized expression, and no decltype. There is a quote from the standard that defines this separately (by now this seems to be expr.cond#4), and there are some differences especially in the prvalue case. So I'm not sure this helps.

[mizvekov]

In D100713#2759637, @aaronpuchert wrote:

Again, I'm not sure if it helps to use getDecltypeForParenthesizedExpr where we don't actually have the decltype of a parenthesized expression.

It's probably not entirely coincidental, but things aren't defined this way.

The current situation with getDecltypeForParenthesizedExpr is that it already is used in some places where there is no decltype of a parenthesized expression either.

This function is a bit new, I introduced it in D98160. I think the mnemonic is OK, but this is the best I could come up at the time. If you have better suggestions, I am all for it.
In the above patch, this is introduced to help out with the type for the expression in a compound requires (http://eel.is/c++draft/expr.prim.req.compound#1.3.2).
But then it is also used in the implementation of decltype itself, where it kicks in even for non-parenthesized expressions (if the expression is not otherwise a special case, as defined in https://eel.is/c++draft/dcl.type.decltype#1).
I think the standard just missed giving a name to this pattern, and it otherwise IMHO misses the mark by using something more complex to explain something simpler.

I think it's no coincidence that for the patch I am working on, I had to change all of these copies in exactly the same way :)

clang/lib/Sema/SemaExprCXX.cpp

5895

Here for XValue and and LValue, the rules are exactly the same as https://eel.is/c++draft/dcl.type.decltype#1.
And the original code here even already special cases it...

I am sure there is some way we can agree that we should not repeat the code just because the standard did not bother to give a name to this part of the rules... And again I think it is no coincidence that it makes sense to perform the same changes in all these cases.

[mizvekov]

rebase.

[rsmith]

clang/lib/AST/ASTContext.cpp

5461

This check doesn't seem like it belongs here.

clang/lib/Sema/SemaExprCXX.cpp

5895

Maybe we can find a more general name for this functionality that doesn't mention decltype. Perhaps something like getReferenceQualifiedType?

[mizvekov]

• rename to getReferenceQualifiedType.
• Move the null expression special case out of the function, back to the original user.

[mizvekov]

clang/lib/Sema/SemaExprCXX.cpp

5895

Yes that is a good name, thanks!

[aaronpuchert]

It is a good name!

clang/include/clang/AST/ASTContext.h

public:
 
   /// Return a ObjCObjectPointerType type for the given ObjCObjectType.
   QualType getObjCObjectPointerType(QualType OIT) const;
 
   /// GCC extension.
   QualType getTypeOfExprType(Expr *e) const;
   QualType getTypeOfType(QualType t) const;
 
+  QualType getReferenceQualifiedType(const Expr *e) const;
+
   /// C++11 decltype.
   QualType getDecltypeType(Expr *e, QualType UnderlyingType) const;
 
   /// Unary type transforms
   QualType getUnaryTransformType(QualType BaseType, QualType UnderlyingType,
                                  UnaryTransformType::UTTKind UKind) const;
 
   /// C++11 deduced auto type.

clang/include/clang/Sema/Sema.h

bool RequireLiteralType(SourceLocation Loc, QualType T, unsigned DiagID,
     BoundTypeDiagnoser<Ts...> Diagnoser(DiagID, Args...);
     return RequireLiteralType(Loc, T, Diagnoser);
   }
 
   QualType getElaboratedType(ElaboratedTypeKeyword Keyword,
                              const CXXScopeSpec &SS, QualType T,
                              TagDecl *OwnedTagDecl = nullptr);
 
-  QualType getDecltypeForParenthesizedExpr(Expr *E);
   QualType BuildTypeofExprType(Expr *E, SourceLocation Loc);
   /// If AsUnevaluated is false, E is treated as though it were an evaluated
   /// context, such as when building a type for decltype(auto).
   QualType BuildDecltypeType(Expr *E, SourceLocation Loc,
                              bool AsUnevaluated = true);
   QualType BuildUnaryTransformType(QualType BaseType,
                                    UnaryTransformType::UTTKind UKind,
                                    SourceLocation Loc);

clang/lib/AST/ASTContext.cpp

 /// on canonical types (which are always unique).
 QualType ASTContext::getTypeOfType(QualType tofType) const {
   QualType Canonical = getCanonicalType(tofType);
   auto *tot = new (*this, TypeAlignment) TypeOfType(tofType, Canonical);
   Types.push_back(tot);
   return QualType(tot, 0);
 }
 
+/// getReferenceQualifiedType - Given an expr, will return the type for
+/// that expression, as in [dcl.type.simple]p4 but without taking id-expressions
+/// and class member access into account.
+QualType ASTContext::getReferenceQualifiedType(const Expr *E) const {
+  // C++11 [dcl.type.simple]p4:
+  //   [...]
+  QualType T = E->getType();
+  switch (E->getValueKind()) {
+  //     - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
+  //       type of e;
+  case VK_XValue:
+    return getRValueReferenceType(T);
+  //     - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
+  //       type of e;
+  case VK_LValue:
+    return getLValueReferenceType(T);
+  //  - otherwise, decltype(e) is the type of e.
+  case VK_PRValue:
+    return T;
+  }
+  llvm_unreachable("Unknown value kind");
+}
+
 /// Unlike many "get<Type>" functions, we don't unique DecltypeType
 /// nodes. This would never be helpful, since each such type has its own
 /// expression, and would not give a significant memory saving, since there
 /// is an Expr tree under each such type.
 QualType ASTContext::getDecltypeType(Expr *e, QualType UnderlyingType) const {
   DecltypeType *dt;
 
   // C++11 [temp.type]p2:

clang/lib/AST/ExprObjC.cpp

QualType ObjCMessageExpr::getCallReturnType(ASTContext &Ctx) const {
   if (const ObjCMethodDecl *MD = getMethodDecl()) {
     QualType QT = MD->getReturnType();
     if (QT == Ctx.getObjCInstanceType()) {
       // instancetype corresponds to expression types.
       return getType();
     }
     return QT;
   }
-
-  // Expression type might be different from an expected call return type,
-  // as expression type would never be a reference even if call returns a
-  // reference. Reconstruct the original expression type.
-  QualType QT = getType();
-  switch (getValueKind()) {
-  case VK_LValue:
-    return Ctx.getLValueReferenceType(QT);
-  case VK_XValue:
-    return Ctx.getRValueReferenceType(QT);
-  case VK_PRValue:
-    return QT;
-  }
-  llvm_unreachable("Unsupported ExprValueKind");
+  return Ctx.getReferenceQualifiedType(this);
 }
 
 SourceRange ObjCMessageExpr::getReceiverRange() const {
   switch (getReceiverKind()) {
   case Instance:
     return getInstanceReceiver()->getSourceRange();
 
   case Class:

clang/lib/Sema/SemaConcept.cpp

case concepts::ExprRequirement::SS_ConstraintsNotSatisfied: {
       ConceptSpecializationExpr *ConstraintExpr =
           Req->getReturnTypeRequirementSubstitutedConstraintExpr();
       if (ConstraintExpr->getTemplateArgsAsWritten()->NumTemplateArgs == 1) {
         // A simple case - expr type is the type being constrained and the concept
         // was not provided arguments.
         Expr *e = Req->getExpr();
         S.Diag(e->getBeginLoc(),
                diag::note_expr_requirement_constraints_not_satisfied_simple)
-            << (int)First << S.getDecltypeForParenthesizedExpr(e)
+            << (int)First << S.Context.getReferenceQualifiedType(e)
             << ConstraintExpr->getNamedConcept();
       } else {
         S.Diag(ConstraintExpr->getBeginLoc(),
                diag::note_expr_requirement_constraints_not_satisfied)
             << (int)First << ConstraintExpr;
       }
       S.DiagnoseUnsatisfiedConstraint(ConstraintExpr->getSatisfaction());
       break;

clang/lib/Sema/SemaExpr.cpp

if (Proto) {
     // This is a hack, but it is far superior to moving the
     // corresponding target-specific code from IR-gen to Sema/AST.
 
     ArrayRef<QualType> ParamTypes = Proto->getParamTypes();
     SmallVector<QualType, 8> ArgTypes;
     if (ParamTypes.empty() && Proto->isVariadic()) { // the special case
       ArgTypes.reserve(E->getNumArgs());
       for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
-        Expr *Arg = E->getArg(i);
-        QualType ArgType = Arg->getType();
-        if (E->isLValue()) {
-          ArgType = S.Context.getLValueReferenceType(ArgType);
-        } else if (E->isXValue()) {
-          ArgType = S.Context.getRValueReferenceType(ArgType);
-        }
-        ArgTypes.push_back(ArgType);
+        ArgTypes.push_back(S.Context.getReferenceQualifiedType(E->getArg(i)));
       }
       ParamTypes = ArgTypes;
     }
     DestType = S.Context.getFunctionType(DestType, ParamTypes,
                                          Proto->getExtProtoInfo());
   } else {
     DestType = S.Context.getFunctionNoProtoType(DestType,
                                                 FnType->getExtInfo());

clang/lib/Sema/SemaExprCXX.cpp

static bool TryClassUnification(Sema &Self, Expr *From, Expr *To,
   //   as follows:
   //   -- If E2 is an lvalue: E1 can be converted to match E2 if E1 can be
   //      implicitly converted to type "lvalue reference to T2", subject to the
   //      constraint that in the conversion the reference must bind directly to
   //      an lvalue.
   //   -- If E2 is an xvalue: E1 can be converted to match E2 if E1 can be
   //      implicitly converted to the type "rvalue reference to R2", subject to
   //      the constraint that the reference must bind directly.
-  if (To->isLValue() || To->isXValue()) {
-    QualType T = To->isLValue() ? Self.Context.getLValueReferenceType(ToType)
-                                : Self.Context.getRValueReferenceType(ToType);
-
+  if (To->isGLValue()) {
+    QualType T = Self.Context.getReferenceQualifiedType(To);
     InitializedEntity Entity = InitializedEntity::InitializeTemporary(T);
 
     InitializationSequence InitSeq(Self, Entity, Kind, From);
     if (InitSeq.isDirectReferenceBinding()) {
       ToType = T;
       HaveConversion = true;
       return false;
     }
else if (ReturnTypeRequirement.isSubstitutionFailure())
     Status = concepts::ExprRequirement::SS_TypeRequirementSubstitutionFailure;
   else if (ReturnTypeRequirement.isTypeConstraint()) {
     // C++2a [expr.prim.req]p1.3.3
     //     The immediately-declared constraint ([temp]) of decltype((E)) shall
     //     be satisfied.
     TemplateParameterList *TPL =
         ReturnTypeRequirement.getTypeConstraintTemplateParameterList();
     QualType MatchedType =
-        getDecltypeForParenthesizedExpr(E).getCanonicalType();
+        Context.getReferenceQualifiedType(E).getCanonicalType();
     llvm::SmallVector<TemplateArgument, 1> Args;
     Args.push_back(TemplateArgument(MatchedType));
     TemplateArgumentList TAL(TemplateArgumentList::OnStack, Args);
     MultiLevelTemplateArgumentList MLTAL(TAL);
     for (unsigned I = 0; I < TPL->getDepth(); ++I)
       MLTAL.addOuterRetainedLevel();
     Expr *IDC =
         cast<TemplateTypeParmDecl>(TPL->getParam(0))->getTypeConstraint()

clang/lib/Sema/SemaType.cpp

QualType Sema::BuildTypeofExprType(Expr *E, SourceLocation Loc) {
   if (!E->isTypeDependent()) {
     QualType T = E->getType();
     if (const TagType *TT = T->getAs<TagType>())
       DiagnoseUseOfDecl(TT->getDecl(), E->getExprLoc());
   }
   return Context.getTypeOfExprType(E);
 }
 
-/// getDecltypeForParenthesizedExpr - Given an expr, will return the type for
-/// that expression, as in [dcl.type.simple]p4 but without taking id-expressions
-/// and class member access into account.
-QualType Sema::getDecltypeForParenthesizedExpr(Expr *E) {
-  // C++11 [dcl.type.simple]p4:
-  //   [...]
-  QualType T = E->getType();
-  switch (E->getValueKind()) {
-  //     - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
-  //       type of e;
-  case VK_XValue:
-    return Context.getRValueReferenceType(T);
-  //     - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
-  //       type of e;
-  case VK_LValue:
-    return Context.getLValueReferenceType(T);
-  //  - otherwise, decltype(e) is the type of e.
-  case VK_PRValue:
-    return T;
-  }
-  llvm_unreachable("Unknown value kind");
-}
-
 /// getDecltypeForExpr - Given an expr, will return the decltype for
 /// that expression, according to the rules in C++11
 /// [dcl.type.simple]p4 and C++11 [expr.lambda.prim]p18.
 static QualType getDecltypeForExpr(Sema &S, Expr *E) {
   if (E->isTypeDependent())
     return S.Context.DependentTy;
 
   Expr *IDExpr = E;
if (isa<ParenExpr>(IDExpr)) {
           QualType T = S.getCapturedDeclRefType(Var, DRE->getLocation());
           if (!T.isNull())
             return S.Context.getLValueReferenceType(T);
         }
       }
     }
   }
 
-  return S.getDecltypeForParenthesizedExpr(E);
+  return S.Context.getReferenceQualifiedType(E);
 }
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
                                  bool AsUnevaluated) {
   assert(!E->hasPlaceholderType() && "unexpected placeholder");
 
   if (AsUnevaluated && CodeSynthesisContexts.empty() &&
       !E->isInstantiationDependent() && E->HasSideEffects(Context, false)) {

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

 using namespace clang;
 using namespace ento;
 
 QualType CallEvent::getResultType() const {
   ASTContext &Ctx = getState()->getStateManager().getContext();
   const Expr *E = getOriginExpr();
   if (!E)
     return Ctx.VoidTy;
-  assert(E);
-
-  QualType ResultTy = E->getType();
-
-  // A function that returns a reference to 'int' will have a result type
-  // of simply 'int'. Check the origin expr's value kind to recover the
-  // proper type.
-  switch (E->getValueKind()) {
-  case VK_LValue:
-    ResultTy = Ctx.getLValueReferenceType(ResultTy);
-    break;
-  case VK_XValue:
-    ResultTy = Ctx.getRValueReferenceType(ResultTy);
-    break;
-  case VK_PRValue:
-    // No adjustment is necessary.
-    break;
-  }
-
-  return ResultTy;
+  return Ctx.getReferenceQualifiedType(E);
 }
 
 static bool isCallback(QualType T) {
   // If a parameter is a block or a callback, assume it can modify pointer.
   if (T->isBlockPointerType() ||
       T->isFunctionPointerType() ||
       T->isObjCSelType())
     return true;