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

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

jdenny added a comment.

In D56113#1345238 <https://reviews.llvm.org/D56113#1345238>, @ABataev wrote:

> >>> By the way, is there any value to keeping the predetermined shared for const if -openmp-version=3.1 or earlier?
> >> 
> >> Yes, you can check for the value of `LangOpts.OpenMP`. For OpenMP 3.1 it will have the value `31`.
> > 
> > How far back should we take this?  I'm inclined to check for `30` and `31` only and assume anything else is newer, but let me know if we need to check for earlier versions.
>
> I think `<= 31` is good. Clang always supported only OpenMP 3.1 and higher.


Ah, for some reason I assumed it was a string not an integer.

In D56113#1345238 <https://reviews.llvm.org/D56113#1345238>, @ABataev wrote:

> In D56113#1345232 <https://reviews.llvm.org/D56113#1345232>, @jdenny wrote:
>
> > In D56113#1345047 <https://reviews.llvm.org/D56113#1345047>, @ABataev wrote:
> >
> > > In D56113#1344210 <https://reviews.llvm.org/D56113#1344210>, @jdenny wrote:
> > >
> > > > In D56113#1342879 <https://reviews.llvm.org/D56113#1342879>, @ABataev wrote:
> > > >
> > > > > But you will need another serie of patches for `reduction` and `linear` clauses to update their error messages.
> > > >
> > > >
> > > > Those already had their own checks for const.  I'm not aware of any cases not handled, and the test suite does pass after my patch.
> > >
> > >
> > > Yes, they have the checks for constness, but you need to  update those checks to emit this new error message for const values with mutable fields.
> >
> >
> > I believe you mean we should reuse `rejectConstNotMutableType` for `reduction` and `linear` rather than leave some of its implementation duplicated for those.
>


For reductions, this will change the message from "const-qualified list item cannot be reduction" to "const-qualified variable cannot be reduction".  Is that OK?  (There are many tests to update, so I want to ask first.)


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

https://reviews.llvm.org/D56113