aaron.ballman added inline comments.

================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4285
+    // created for us by the caller.
+    return true;
+  }
----------------
erichkeane wrote:
> ABataev wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > Another place to make the same comment :D  I wonder if giving a 
> > > > > diagnostic on attempting to use omp::directive in a previous version 
> > > > > should have either an extension warning or more explicit warning 
> > > > > would be a good idea?
> > > > Ah, now I see what you're after. We could do that, but I think deciding 
> > > > whether to expose this in older modes is more of an open question. I 
> > > > went with the conservative approach first, but we could relax this 
> > > > later.
> > > Well, I was going for 1 of two things:
> > > 
> > > 1- allow in older modes, then do a warning that you're using it in the 
> > > wrong version.
> > > 
> > > 2- Customize the error from "unknown attribute directive" to something 
> > > like, "attribute omp::directive is only available in OMP5.1 mode".
> > I think we can enable it for the previous versions, at least as an 
> > extension. Thoughts?
> It'll mean a larger test-surface I'd think, but there _IS_ precedent both 
> ways.  
> IMO, we are probably better off doing #2 above (a better 'unknown attribute' 
> diagnostic), then doing it as an extension if GCC makes that choice, or there 
> is a compelling reaosn.
I think it makes sense to improve the diagnostic here, but I think we should 
hold off on enabling it as an extension in older OpenMP modes until we have 
some usage experience with the feature. Once we have confidence that the 
feature works well in 5.1, then it's easy enough for us to support it in older 
modes.


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

https://reviews.llvm.org/D105648

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to