[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:9736
 
+static bool rejectConstNotMutableType(Sema &SemaRef, ValueDecl *D,
+                                      OpenMPClauseKind CKind,
----------------
ABataev wrote:
> ABataev wrote:
> > This function and the original code has a lot of common code. Could you, please, use this function somehow for the original too? Or outline the common code into standalone function
> Also, probably missed the check if the OpenMP version is >= 4.0
> This function and the original code has a lot of common code. Could you, please, use this function somehow for the original too? Or outline the common code into standalone function

Sure, I'll work on that.

> Also, probably missed the check if the OpenMP version is >= 4.0

This is what I was trying to explain in my last comment: I now realize that 3.1 also has the const restriction for private and lastprivate.  For example:

> A variable that appears in a private clause must not have a const-qualified type
> unless it is of class type with a mutable member. 

To make it clear that this isn't limited to 4.0 and later, perhaps I should also quote 3.1 at the calls to  rejectConstNotMutableType.

However, clang didn't previously implement those restrictions for private and lastprivate probably because the check for conflicts with predetermined shared was sufficient to catch those cases.

So, for 3.1, it should be correct for clang to report either diagnostic, but again I think the new diagnostic is clearer. For 4.0 and later, only the new diagnostic makes sense.  So, my patch makes clang report the new diagnostic for all OpenMP versions.

Of course, I can adjust if this doesn't make sense to you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56113