aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:8274-8279
// Only if a local variable was declared in the function currently being
// evaluated, do we expect to be able to find its value in the current
// frame. (Otherwise it was likely declared in an enclosing context and
// could either have a valid evaluatable value (for e.g. a constexpr
// variable) or be ill-formed (and trigger an appropriate evaluation
// diagnostic)).
----------------
jdoerfert wrote:
> aaron.ballman wrote:
> > This comment no longer matches the code. It also suggests that the way to
> > address your issue is to capture the variable into the current frame -- did
> > you explore that approach?
> The problem is that OpenMP introduces artificial capture statements. I am not
> sure if/how those can be "merged" into the parent context and in general that
> is not allowed (e.g., as we allow clauses on SIMD, like private). I feel the
> comment needs updating and I'll try that.
Is there a way to limit the change to only OpenMP captured variables so that
we're not changing the semantics for all language modes?
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2175-2178
+ SemaRef.Diag(
+ S->getBeginLoc(),
+ diag::err_constexpr_body_invalid_omp_simd_stmt_with_clauses)
+ << isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval();
----------------
jdoerfert wrote:
> aaron.ballman wrote:
> > C++20 now has constexpr destructors, but there are also other kinds of
> > special member functions too. It seems like the constraint really is
> > "constant evaluation of a function with SIMD clauses is invalid" and so we
> > might want to go with a more generic diagnostic instead of trying to
> > distinguish between kinds of functions.
> I copied this from below (see comment). I would assume the two have to stay
> in sync. All the differentiation of constructor does it to specify where it
> was found, no? So:
> SIMD with clauses is not supported, found in XYZ.
Yeah, they should stay in sync; I don't insist on a change to the diagnostic as
part of this review unless you feel like it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135199/new/
https://reviews.llvm.org/D135199
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
