Hal, thanks for the review!

================
Comment at: lib/Sema/SemaOpenMP.cpp:3137
@@ +3136,3 @@
+    // value. The address of the original variable is replaced by the address 
of
+    // the new private variable in the CodeGen.This new variable is not added 
to
+    // IdResolver, so the code in the OpenMP region uses original variable for
----------------
hfinkel wrote:
> in the CodeGen -> in CodeGen
> Missing space before "This"
Ok, fixed.

================
Comment at: lib/Sema/SemaOpenMP.cpp:3139
@@ +3138,3 @@
+    // IdResolver, so the code in the OpenMP region uses original variable for
+    // proper diagnostics and variable capturing.
+    auto VDPrivate =
----------------
hfinkel wrote:
> Also, "proper diagnostics" I understand (we don't want diagnostic messages 
> saying that variables are declared inside of OpenMP clauses -- that would get 
> confusing). I don't understand what you mean here by "variable capturing", 
> but I also thing the point is: how much do we want the presence of the OpenMP 
> clauses to affect the AST, and regarding adding an extra layer of indirection 
> for all variables inside the clause when -fopenmp is enabled, I think it 
> makes sense that we don't want that.
Agree. I'm thinking on small redesign of var capturing.

http://reviews.llvm.org/D4752



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to