[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked an inline comment as done.
gribozavr added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5784
     Expr::EvalResult Result;
-    if (CollapseLoopCountExpr->EvaluateAsInt(Result, SemaRef.getASTContext()))
+    if (!CollapseLoopCountExpr->isValueDependent() &&
+        !CollapseLoopCountExpr->isTypeDependent() &&
----------------
ABataev wrote:
> I would suggest to modify the code of this function if we cannot get the value of the loops.
> ```
> if (CollapseLoopCountExpr->isValueDependent() || CollapseLoopCountExpr->isTypeDependent() || OrderedLoopCountExpr->isValueDependent() || OrderedLoopCountExpr->isTypeDependent()) {
>   Built.clear(/* size */0);
>   return 1;
> }
> ```
> at the beginning of the function.
I tried doing that, and a lot of tests started crashing with:

llvm-project/clang/lib/Sema/SemaOpenMP.cpp:9024: clang::StmtResult clang::Sema::ActOnOpenMPTargetTeamsDistributeSimdDirective(ArrayRef<clang::OMPClause *>, clang::Stmt *, clang::SourceLoc
ation, clang::SourceLocation, clang::Sema::VarsWithInheritedDSAType &): Assertion `(CurContext->isDependentContext() || B.builtAll()) && "omp target teams distribute simd loop exprs were not built"' failed.

Also, I wanted to note that if I were to make this change, then if `EvaluateAsInt` fails, we should apply the same recovery (`Built.clear(); return`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61522