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 CompCategories;
----------------
We don't generally indent comments after a `\brief` like this. Also, we enable
autobrief, so these `\brief`s are redundant.
================
Comment at: include/clang/AST/ComparisonCategories.h:56
+/// comparison. These values map onto instances of comparison category types
+/// defined in the standard library. i.e. 'std::strong_ordering::less'.
+enum class ComparisonCategoryResult : unsigned char {
----------------
You mean "eg", not "ie" here.
================
Comment at: include/clang/AST/ComparisonCategories.h:84-103
+ /// \brief True iff we've successfully evaluated the variable as a constant
+ /// expression and extracted its integer value.
+ bool hasValidIntValue() const { return HasValue; }
+
+ /// \brief Get the constant integer value used by this variable to
represent
+ /// the comparison category result type.
+ llvm::APSInt getIntValue() const {
----------------
This seems unnecessary; we can get this information from the `VarDecl` instead.
(You're caching a result here that is already cached there.)
================
Comment at: lib/AST/ComparisonCategories.cpp:25
+/// category result by evaluating the initializer for the specified VarDecl as
+/// a constant expression and retreiving the value of the classes first
+/// (and only) field.
----------------
classes -> class's
================
Comment at: lib/AST/ComparisonCategories.cpp:43-46
+ Expr::EvalResult Result;
+ if (!Info->VD->hasInit() ||
+ !Info->VD->getInit()->EvaluateAsRValue(Result, Ctx))
+ return true;
----------------
Use `VD->evaluateValue()` to get the cached value already stored by the
`VarDecl`.
================
Comment at: lib/CodeGen/CGExprAgg.cpp:959
+ !ArgTy->isMemberPointerType() && !ArgTy->isAnyComplexType()) {
+ return CGF.ErrorUnsupported(E, "aggregate three-way comparisoaoeun");
+ }
----------------
Typo. I'm almost tempted to say we should keep this for entertainment value,
but on balance let's fix it :)
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2899-2937
assert(!location.getAs<NonLoc>() && "location cannot be a NonLoc.");
+
+ // Are we loading from a region? This actually results in two loads; one
+ // to fetch the address of the referenced value and one to fetch the
+ // referenced value.
+ if (const auto *TR =
+ dyn_cast_or_null<TypedValueRegion>(location.getAsRegion())) {
----------------
This does not look related to your three-way comparison changes.
================
Comment at: test/SemaCXX/compare-cxx2a.cpp:40
-#if 0
+ (void)(A < 42);
// (A,b)
----------------
Did you intend to add this here? It doesn't look related to the code under test.
================
Comment at: test/SemaCXX/std-compare-cxx2a.cpp:6
+void compare_not_found_test() {
+ // expected-error@+1 {{cannot deduce return type of 'operator<=>' because
type partial_ordering was not found; include <compare>}}
+ (void)(0.0 <=> 42.123);
----------------
This diagnostic says just `partial_ordering` whereas the one below says
`'std::partial_ordering'`. I prefer the latter more-explicit form, but in any
case it would seem good to be consistent.
Repository:
rL LLVM
https://reviews.llvm.org/D45476
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
