Topotuna added inline comments.

================
Comment at: clang/lib/Sema/SemaType.cpp:1729
     bool IsOpenCLC30 = (S.getLangOpts().OpenCLVersion == 300);
+    bool IsOpenCLC30Comp = S.getLangOpts().getOpenCLCompatibleVersion() == 300;
     // OpenCL C v3.0 s6.3.3 - OpenCL image types require __opencl_c_images
----------------
Topotuna wrote:
> Anastasia wrote:
> > Topotuna wrote:
> > > Anastasia wrote:
> > > > I think we should just replace `IsOpenCLC30` as it is when only used in 
> > > > the diagnostic that we should report the same as for OpenCL 3.0.
> > > > 
> > > > 
> > > > Also I would suggest changing name to `IsOpenCLC30Compatible` to make 
> > > > it clearer. 
> > > That diagnostic is responsible for `__opencl_c_3d_image_writes` feature 
> > > while current commit addresses `__opencl_c_images`. I wanted to keep 
> > > commits separate and was planning to finish transitioning from 
> > > `IsOpenCLC30` to `IsOpenCLC30Compatible` in a future commit.
> > > 
> > > I agree that `IsOpenCLC30Compatible` would be clearer. Thank you.
> > Ok, let's just only rename it for now and add a FIXME comment explaining 
> > that the variables should be unified later on.
> > 
> > I suspect your subsequent commits would include the unification?
> Yes, I will perform unification when adding support for 
> `__opencl_c_3d_image_writes` optional core feature.
Change rGca3bebd8440f performs mentioned variable unification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109002

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

Reply via email to