rsmith added inline comments.
Comment at: include/clang/AST/ASTContext.h:1983
+ /// \brief Types and expressions required to build C++2a three-way
comparisons
+ /// using operator<=>, including the values return by builtin <=>
operators.
+ ComparisonCategories CompCategori
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Fine, that works.
https://reviews.llvm.org/D45476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
EricWF added inline comments.
Comment at: lib/AST/ExprConstant.cpp:8829
+ return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+ });
rsmith wrote:
> EricWF wrote:
> > rsmith wrote:
> > > It
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:964
+RHS = CGF.EmitAnyExpr(E->getRHS()).getAggregatePointer();
+break;
+ case TEK_Complex:
EricWF wrote:
> EricWF wrote:
> > rjmccall wrote:
> > > It looks like we don't actually suppo
rsmith added inline comments.
Comment at: lib/AST/ExprConstant.cpp:8829
+ return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+ });
EricWF wrote:
> rsmith wrote:
> > It'd be clearer to cal
EricWF added inline comments.
Comment at: lib/AST/ExprConstant.cpp:8829
+ return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+ });
rsmith wrote:
> It'd be clearer to call `VisitBinCmp` ra
EricWF added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:964
+RHS = CGF.EmitAnyExpr(E->getRHS()).getAggregatePointer();
+break;
+ case TEK_Complex:
EricWF wrote:
> rjmccall wrote:
> > It looks like we don't actually support any aggregate types
EricWF marked 8 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:71
+ /// standard library. The key is a value of ComparisonCategoryResult.
+ mutable llvm::DenseMap Objects;
+
rjmccall wrote:
> EricWF w
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:964
+RHS = CGF.EmitAnyExpr(E->getRHS()).getAggregatePointer();
+break;
+ case TEK_Complex:
It looks like we don't actually support any aggregate types here, which I think
is fine beca
rjmccall added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:71
+ /// standard library. The key is a value of ComparisonCategoryResult.
+ mutable llvm::DenseMap Objects;
+
EricWF wrote:
> rjmccall wrote:
> > We expect this map to have at
EricWF marked 12 inline comments as done.
EricWF added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:9825
+LHS = S.ImpCastExprToType(LHS.get(), IntType, CK_IntegralCast);
+RHS = S.ImpCastExprToType(RHS.get(), IntType, CK_IntegralCast);
+LHSType = RHSType = IntTyp
EricWF marked 18 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/ASTContext.h:1986
+ /// This object needs to be initialized by Sema the first time it checks
+ /// a three-way comparison.
+ ComparisonCategories CompCategories;
-
rjmccall added inline comments.
Comment at: include/clang/AST/ASTContext.h:1986
+ /// This object needs to be initialized by Sema the first time it checks
+ /// a three-way comparison.
+ ComparisonCategories CompCategories;
Is this comment accurate? Because i
EricWF marked an inline comment as done.
EricWF added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:971
+ auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+ };
rsmith wrote:
> Perhaps directly emit the constant val
rsmith added inline comments.
Comment at: lib/AST/ExprConstant.cpp:8829
+ return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+ });
It'd be clearer to call `VisitBinCmp` rather than `Visit
rjmccall added a comment.
In https://reviews.llvm.org/D45476#1088189, @rsmith wrote:
> This is significantly more complexity than we need. We're talking about
> constexpr variables here, so we just need a `VarDecl* -- you can then ask
> that declaration for its evaluated value and emit that dir
rjmccall added a comment.
In https://reviews.llvm.org/D45476#1088180, @EricWF wrote:
> OK, As I see it, we have two choices:
>
> (1) Stash the expressions in Sema and import them like
>
> In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote:
>
> > I'm sorry, but this is just not true. The
rsmith added a comment.
This is significantly more complexity than we need. We're talking about
constexpr variables here, so we just need a `VarDecl* -- you can then ask that
declaration for its evaluated value and emit that directly.
https://reviews.llvm.org/D45476
EricWF added a comment.
OK, As I see it, we have two choices:
(1) Stash the expressions in Sema and import them like
In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote:
>
> Ah. If you want to be able to find this thing without a Sema around in order
> to
> handle deserialized ex
rjmccall added a comment.
In https://reviews.llvm.org/D45476#1087487, @EricWF wrote:
> In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:
>
> > I think you and Richard agreed that you weren’t going to synthesize a whole
> > expression tree at every use of the operator, and I agree w
Woops. Submitted that last comment too early. Editing it on Phab.
On Fri, May 4, 2018 at 2:31 AM, Eric Fiselier via Phabricator <
revi...@reviews.llvm.org> wrote:
> EricWF added a comment.
>
> In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:
>
> > I think you and Richard agreed tha
EricWF added a comment.
In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:
> I think you and Richard agreed that you weren’t going to synthesize a whole
> expression tree at every use of the operator, and I agree with that
> decision. That’s very different from what I’m asking you
I think you and Richard agreed that you weren’t going to synthesize a whole
expression tree at every use of the operator, and I agree with that
decision. That’s very different from what I’m asking you to do, which is to
synthesize in isolation a call to the copy-constructor. There are several
place
cfe-commits added a subscriber: junbuml.
cfe-commits added a comment.
I think you and Richard agreed that you weren’t going to synthesize a whole
expression tree at every use of the operator, and I agree with that
decision. That’s very different from what I’m asking you to do, which is to
synthesi
EricWF added inline comments.
Comment at: lib/Sema/SemaDeclCXX.cpp:8931
+ /*ConstArg*/ true, false, false, false, false);
+ auto *CopyCtor = cast_or_null(SMI.getMethod());
+
rjmccall wrote:
> Sorry, I didn't realize you'd go off and writ
rjmccall added inline comments.
Comment at: lib/Sema/SemaDeclCXX.cpp:8931
+ /*ConstArg*/ true, false, false, false, false);
+ auto *CopyCtor = cast_or_null(SMI.getMethod());
+
Sorry, I didn't realize you'd go off and write this code manu
EricWF added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+ return EmitFinalDestCopy(
+ E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}
EricWF wrote:
> rjmccall wrote:
> > Is there something in Sema actually validating
EricWF added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+ return EmitFinalDestCopy(
+ E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}
rjmccall wrote:
> Is there something in Sema actually validating that the comparis
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+ return EmitFinalDestCopy(
+ E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}
Is there something in Sema actually validating that the comparison types is
tri
rsmith added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:9816
+RHS = S.ImpCastExprToType(RHS.get(), Type, CK_BitCast);
+ } else {
+// C++2a [expr.spaceship]p4
EricWF wrote:
> rsmith wrote:
> > We still need to apply the usual arithmetic conversion
EricWF marked 3 inline comments as done.
EricWF added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:9816
+RHS = S.ImpCastExprToType(RHS.get(), Type, CK_BitCast);
+ } else {
+// C++2a [expr.spaceship]p4
rsmith wrote:
> We still need to apply the usua
EricWF added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:9928
+// result is of type std::strong_equality
+if (CompositeTy->isFunctionPointerType() ||
+CompositeTy->isMemberPointerType() || CompositeTy->isNullPtrType())
EricWF wrote:
> rsmit
EricWF marked 4 inline comments as done.
EricWF added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:9928
+// result is of type std::strong_equality
+if (CompositeTy->isFunctionPointerType() ||
+CompositeTy->isMemberPointerType() || CompositeTy->isNullPtrType(
EricWF marked 24 inline comments as done.
EricWF added inline comments.
Comment at: lib/AST/ExprConstant.cpp:3784
+static bool EvaluateVarDecl(EvalInfo &Info, const VarDecl *VD,
+APValue *Dest = nullptr) {
// We don't need to evaluate the initialize
rsmith added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:9928
+// result is of type std::strong_equality
+if (CompositeTy->isFunctionPointerType() ||
+CompositeTy->isMemberPointerType() || CompositeTy->isNullPtrType())
Please add a FIXME he
rsmith added a comment.
Thanks! This is looking pretty close.
Comment at: include/clang/AST/ComparisonCategories.h:78
+public:
+ /// \brief Wether Sema has fully checked the type and result values for this
+ /// comparison category types before.
Typo "Wethe
EricWF marked 30 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:194
+
+ const ASTContext &Ctx;
+ mutable llvm::DenseMap Data;
Storing the `ASTContext` in `ComparisonCategories` and `ComparisonCategory
rsmith added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:77-78
+ /// comparison category. For example 'std::strong_equality::equal'
+ const DeclRefExpr *getResultValue(ComparisonCategoryResult ValueKind) const {
+const DeclRefExpr *DR = getResult
EricWF added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:77-78
+ /// comparison category. For example 'std::strong_equality::equal'
+ const DeclRefExpr *getResultValue(ComparisonCategoryResult ValueKind) const {
+const DeclRefExpr *DR = getResult
EricWF added inline comments.
Comment at: lib/Sema/SemaDeclCXX.cpp:
+bool Sema::BuildComparisonCategoryData(SourceLocation Loc) {
+ using CCKT = ComparisonCategoryKind;
rsmith wrote:
> EricWF wrote:
> > rsmith wrote:
> > > If you put this on Sema, you'll n
EricWF added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9384-9385
+def err_spaceship_comparison_of_void_ptr : Error<
+ "three-way comparison with void pointer %select{operand type|operand types}0
"
+ "(%1 and %2)">;
+def err_spaceship_comparison_of
rsmith added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:77-78
+ /// comparison category. For example 'std::strong_equality::equal'
+ const DeclRefExpr *getResultValue(ComparisonCategoryResult ValueKind) const {
+const DeclRefExpr *DR = getResult
EricWF marked 7 inline comments as done.
EricWF added inline comments.
Comment at: lib/AST/ExprConstant.cpp:6238-6263
+bool VisitBinCmp(const BinaryOperator *E) {
+ using CCR = ComparisonCategoryResult;
+ const ComparisonCategoryInfo &CmpInfo =
+ Info.Ctx.C
EricWF marked 5 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9377-9378
+def err_implied_comparison_category_type_not_found : Error<
+ "%0 type was not found; include before defining "
+ "or using 'operator<=>'">;
EricWF marked 4 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/Expr.h:3097-3106
+ bool isRelationalOp() const {
+return isRelationalOp(getOpcode()) ||
+ (getOpcode() == BO_Cmp && IsCmpOrdered);
+ }
static bool isEqualityO
EricWF marked 4 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:68
+ /// \brief A map containing the comparison category values built from the
+ /// standard library. The key is a value of ComparisonCategoryValue.
+ l
rsmith added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:68
+ /// \brief A map containing the comparison category values built from the
+ /// standard library. The key is a value of ComparisonCategoryValue.
+ llvm::DenseMap Objects;
W
EricWF added inline comments.
Comment at: include/clang/AST/ComparisonCategories.h:155-158
+ const ComparisonCategoryInfo &getInfo(ComparisonCategoryKind Kind) const {
+assert(HasData && "comparison category information not yet built");
+return Data[static_cast(Kind)];
+
aaron.ballman added a comment.
Thank you for working on this!
Comment at: include/clang/AST/ComparisonCategories.h:78
+ const DeclRefExpr *getResultValue(ComparisonCategoryResult ValueKind) const {
+const auto *DR = getResultValueUnsafe(ValueKind);
+assert(DR &&
--
49 matches
Mail list logo