riccibruno added a comment. In D52879#1314694 <https://reviews.llvm.org/D52879#1314694>, @mantognini wrote:
> In D52879#1311177 <https://reviews.llvm.org/D52879#1311177>, @riccibruno > wrote: > > > And moreover I believe this change is subtly incorrect for the following > > reason: > > The type that is passed into the constructor of the call expression is the > > type > > of the call expression, and not the return type. > > > > The difference between the return type and the type of the call expression > > is that > > 1.) References are dropped, and > > 2.) For C++: The cv-qualifiers of non class pr-values are dropped, > > 3.) For C: The cv-qualifiers are dropped. > > > > ( as can be seen in `FunctionType::getCallResultType` ) > > > Could you clarify one thing for me: If I understood what you were saying, the > type passed to CallExpr should not be ref-cv-qualified, is that right? > > In that case, couldn't we "simply" remove the ref + CV-qualifiers from > ReturnTy? (`getNonReferenceType().withoutLocalFastQualifiers()`) > > Note: I cannot revert this //and// keep the test because the new ones will > fail. It's not that simple unfortunately. The type passed to `CallExpr` is the type of the call expression, which is different from the return type. However you can just use `FunctionType::getCallResultType`. ================ Comment at: lib/Sema/SemaExpr.cpp:5550 ExprResult Result; + QualType ReturnTy; if (BuiltinID && ---------------- Please rename this to something more appropriate, like `ResultTy` or similar. ================ Comment at: lib/Sema/SemaExpr.cpp:5553 Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) { - Result = ImpCastExprToType(Fn, Context.getPointerType(FDecl->getType()), - CK_BuiltinFnToFnPtr).get(); + // Extract the return type from the (builtin) function pointer type. + auto FnPtrTy = Context.getPointerType(FDecl->getType()); ---------------- Please leave a FIXME indicating that someone should go through each of the builtins and remove the various `setType` when appropriate. ================ Comment at: lib/Sema/SemaExpr.cpp:5554 + // Extract the return type from the (builtin) function pointer type. + auto FnPtrTy = Context.getPointerType(FDecl->getType()); + Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get(); ---------------- Don't use `auto` here. ================ Comment at: lib/Sema/SemaExpr.cpp:5556 + Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get(); + auto FnTy = FnPtrTy->getPointeeType()->castAs<FunctionType>(); + ReturnTy = FnTy->getReturnType(); ---------------- `const auto *` or just spell the type. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52879/new/ https://reviews.llvm.org/D52879 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
