[PATCH] D64356: [OPENMP]Initial fix PR42392: Improve -Wuninitialized warnings for OpenMP programs.

Mon Jul 8 15:45:50 PDT 2019

ABataev marked an inline comment as done.
ABataev added a comment.

In D64356#1574540 <https://reviews.llvm.org/D64356#1574540>, @NoQ wrote:

> I don't know much about OpenMP, but i guess i can help with the CFG a bit.

Thanks a lot!

> I strongly recommend adding a few direct tests for the CFG so that to check that the order of statements and the overall topology of the CFG is as you intended. Currently those tests are routed through the Static Analyzer because nobody else seems to care; see `test/Analysis/cfg.cpp` as an example (there are a few more such tests, grep for `DumpCFG`).

Thanks for the advice, will do.

> I tried to apply the patch and accidentally noticed that none of the newly added tests actually pass for me. Is this the right patch? Or is it just me?

That's strange. Maybe the patch applied incorrectly?

================
Comment at: lib/Analysis/CFG.cpp:4746-4750
+  D->for_each_used_expr([this, &B](Expr *E) {
+    assert(E && "Expected expression.");
+    if (CFGBlock *R = Visit(E))
+      B = R;
+  });
----------------
NoQ wrote:
> Not sure, are these expressions actually evaluated in run-time when the directive is hit? When i looked at a few AST dumps it seemed to me as if these are just a few `DeclRefExpr`s that refer back to the captured variables; they don't really do anything.
Some of them are real expressions, evaluated at the runtime. 

Some of them are the captured variables. But for some of those vars we need to create private copies, initialized with the values of the original variables. And we need to diagnose that the original captured variable is used without being initialized.
I'm going to extend this function to check all required expressions later. This is just an initial patch to setup the basic required functionality.

Repository:
  rC Clang

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

https://reviews.llvm.org/D64356