tahonermann added inline comments.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:377-380
+ SubstitutedExpression = ImplicitCastExpr::Create(
+ S.Context, SubstitutedExpression.get()->getType(),
+ CK_LValueToRValue, SubstitutedExpression.get(),
+ /*BasePath=*/nullptr, VK_PRValue, FPOptionsOverride());
----------------
`ImplicitCastExpr::Create()` has the following assertion. I wonder if we need
to guard against this here.
clang/lib/AST/Expr.cpp:
2073 assert((Kind != CK_LValueToRValue ||
2074 !(T->isNullPtrType() || T->getAsCXXRecordDecl())) &&
2075 "invalid type for lvalue-to-rvalue conversion");
Or perhaps it would be better to call `Sema::DefaultLvalueConversion()` here?
================
Comment at: clang/lib/Sema/SemaConcept.cpp:383
if (!S.CheckConstraintExpression(SubstitutedExpression.get()))
- return ExprError();
+ return ExprError(); // convert to bool here?
----------------
This comment appears to be unnecessary.
================
Comment at:
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp:3-4
+
+template<typename T> concept C =
+sizeof(T) == 4 && !true; // requires atomic constraints sizeof(T) == 4
and !true
+
----------------
I'm not sure what this is intending to exercise. `C` isn't used elsewhere; this
just appears to validate that a well-formed concept declaration is accepted
(even if it is one that will always evaluate as false). I suggest modifying it
to drop `== 4 && !true` and then validate that a diagnostic is issued (which
might require a use of the concept).
================
Comment at:
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp:28
+ f2(0); // #F2INST
+}
----------------
These tests look good.
I think we should also exercise nested requirements to ensure that behavior is
consistent:
template<typename T> requires requires {
requires S<T>{};
}
void f3(T);
void f3(int);
Perhaps also add some examples where `&&` does not denote a conjunction of
atomic constraints. For example:
template<typename T> requires (bool(1 && 2))
void f4(T);
void f4(int);
================
Comment at: libcxx/include/module.modulemap.in:750-753
+ module common_reference_with {
+ private header "__concepts/common_reference_with.h"
+ export type_traits.common_reference
+ }
----------------
I agree with this being done in a separate commit.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141954/new/
https://reviews.llvm.org/D141954
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
