[PATCH] D98160: [clang] Use decltype((E)) for compound requirement type constraint

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/lib/Sema/SemaType.cpp:8838
+/// that expression, without accounting for the rules.
+static QualType getBaseDecltypeForExpr(Sema &S, Expr *E) {
+  // C++11 [dcl.type.simple]p4:
----------------
I think a better name for this might be `getDecltypeForNonIdentifierExpr` and/or `getDecltypeForParenthesizedExpr`.
Depending on the mental model you want people to have, `ImplicitParens` might be better named `AsNonIdentifier`.
The thing we're enabling/disabling here is "whether to treat an //id-expression// differently from any other kind of expression."

Also, FYI and FWIW, I have a proposal in the pipeline for C++23 that will make "the decltype of" the //id-expression// in `return x` into an rvalue (for the purposes of `decltype(auto)` return types). So it might be worth thinking about what that codepath would look like, since it's another example of "treating //id-expressions// differently from other kinds of expressions (but in a different way from how we already do)."
http://quuxplusone.github.io/draft/d2266-implicit-move-rvalue-ref.html
If I ran the zoo, I'd have a primitive function for "get the decltype of this expression, ignoring all crazy //id-expression// rules," and then I'd layer various helper functions on top of it that did, like, "if (is id-expression) craziness; else call the primitive function."

Finally, your comment on line 8837 ought to indicate //what// rules it's not accounting for. "The rules" is pretty vague. ;)


================
Comment at: clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp:97
 template<typename T>
-concept Large = sizeof(T) >= 4; // expected-note{{because 'sizeof(short) >= 4' (2 >= 4) evaluated to false}}
+concept LargeRef = sizeof(typename reference<T>::type) >= 4;
+// expected-note at -1{{because 'sizeof(typename reference<short &>::type) >= 4' (2 >= 4) evaluated to false}}
----------------
I think this is too large of a change. How about just keeping the old test code but changing it to
```
template<typename T>
concept Large = sizeof(std::decay_t<T>) >= 4; // expected-note{{because... etc}}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98160