Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-07-19 Thread Anastasia Stulova via cfe-commits 
Anastasia closed this revision.
Anastasia added a comment.

r269305


https://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-27 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

In http://reviews.llvm.org/D19780#442305, @rivanvx wrote:

> @Anastasia I looked into introducing a separate getOpenCLVersion() function 
> (or perhaps three - major version, minor version and version string). This 
> would have to be used in lib/CodeGen/TargetInfo.cpp and 
> lib/Parse/ParseDecl.cpp, and I am undecided on where should one put this 
> code. One option would be in Parse/Parser.h inside class Parser, and then 
> TargetInfo.cpp would have to include Parser.h, unless we decide to declare it 
> inside AST/ASTContext.h.
>
> In any case, this has so far two usages, and they are different (major and 
> minor version in TargetInfo.cpp vs version string in ParseDecl.cpp). 
> Therefore, I would propose to leave this as is for now, and rethink it after 
> the same code has to be used in more places.


Sure! It seems to make sense to me to have more use cases first. ;) Thanks for 
looking at it!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-27 Thread Vedran Miletić via cfe-commits 
rivanvx added a comment.

@Anastasia I looked into introducing a separate getOpenCLVersion() function (or 
perhaps three - major version, minor version and version string). This would 
have to be used in lib/CodeGen/TargetInfo.cpp and lib/Parse/ParseDecl.cpp, and 
I am undecided on where should one put this code. One option would be in 
Parse/Parser.h inside class Parser, and then TargetInfo.cpp would have to 
include Parser.h, unless we decide to declare it inside AST/ASTContext.h.

In any case, this has so far two usages, and they are different (major and 
minor version in TargetInfo.cpp vs version string in ParseDecl.cpp). Therefore, 
I would propose to leave this as is for now, and rethink it after the same code 
has to be used in more places.


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-12 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

Sure. Committed in r269305!

Thanks!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-11 Thread Vedran Miletić via cfe-commits 
rivanvx added a comment.

Could we solve that at a later point? There is one more place where such code 
is already used, but this would enlarge the scope of this patch.

If yes, I am wiling to factor it out after this is merged.


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-11 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

Sure! Will do! Thanks!

I am thinking to factor out the version computation string into a common 
function, because we might use it in the other places too.


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-11 Thread Vedran Miletić via cfe-commits 
rivanvx added a comment.

Please, can anyone push this?


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-09 Thread Vedran Miletić via cfe-commits 
rivanvx added a comment.

Thanks for the reviews!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-09 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

Please, move cfe-commits to Subscribers list!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-09 Thread Anastasia Stulova via cfe-commits 
Anastasia accepted this revision.
Anastasia added a comment.

LGTM! Thanks!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-08 Thread Xiuli PAN via cfe-commits 
pxli168 accepted this revision.
pxli168 added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-07 Thread Vedran Miletić via cfe-commits 
rivanvx updated this revision to Diff 56479.
rivanvx added a comment.

Make that int const as well.


http://reviews.llvm.org/D19780

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParseDecl.cpp
  test/Parser/opencl-cl20.cl
  test/Parser/opencl-storage-class.cl
  test/SemaOpenCL/invalid-access-qualifier.cl
  test/SemaOpenCL/storageclass.cl

Index: test/SemaOpenCL/storageclass.cl
===
--- test/SemaOpenCL/storageclass.cl
+++ test/SemaOpenCL/storageclass.cl
@@ -13,7 +13,7 @@
   constant int L1 = 0;
   local int L2;
 
-  auto int L3 = 7; // expected-error{{OpenCL does not support the 'auto' storage class specifier}}
+  auto int L3 = 7; // expected-error{{OpenCL version 1.2 does not support the 'auto' storage class specifier}}
   global int L4;   // expected-error{{function scope variable cannot be declared in global address space}}
 }
 
Index: test/SemaOpenCL/invalid-access-qualifier.cl
===
--- test/SemaOpenCL/invalid-access-qualifier.cl
+++ test/SemaOpenCL/invalid-access-qualifier.cl
@@ -10,5 +10,5 @@
 #ifdef CL20
 void test4(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'pipe int'}}
 #else
-void test4(__read_write image1d_t i) {} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' earlier than OpenCL2.0 version}}
+void test4(__read_write image1d_t i) {} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' earlier than OpenCL version 2.0}}
 #endif
Index: test/Parser/opencl-storage-class.cl
===
--- test/Parser/opencl-storage-class.cl
+++ test/Parser/opencl-storage-class.cl
@@ -2,10 +2,10 @@
 
 void test_storage_class_specs()
 {
-  static int a;// expected-error {{OpenCL does not support the 'static' storage class specifier}}
-  register int b;  // expected-error {{OpenCL does not support the 'register' storage class specifier}}
-  extern int c;// expected-error {{OpenCL does not support the 'extern' storage class specifier}}
-  auto int d;  // expected-error {{OpenCL does not support the 'auto' storage class specifier}}
+  static int a;// expected-error {{OpenCL version 1.0 does not support the 'static' storage class specifier}}
+  register int b;  // expected-error {{OpenCL version 1.0 does not support the 'register' storage class specifier}}
+  extern int c;// expected-error {{OpenCL version 1.0 does not support the 'extern' storage class specifier}}
+  auto int d;  // expected-error {{OpenCL version 1.0 does not support the 'auto' storage class specifier}}
 
 #pragma OPENCL EXTENSION cl_clang_storage_class_specifiers : enable
   static int e; // expected-error {{static local variable must reside in constant address space}}
Index: test/Parser/opencl-cl20.cl
===
--- test/Parser/opencl-cl20.cl
+++ test/Parser/opencl-cl20.cl
@@ -10,17 +10,17 @@
   return var;  
 }
 #ifndef CL20
-// expected-error@-5 {{OpenCL does not support the '__generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the '__generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the '__generic' type qualifier}}
+// expected-error@-5 {{OpenCL version 1.0 does not support the '__generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the '__generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the '__generic' type qualifier}}
 #endif
 
 generic int * generic_test(generic int *arg) {
   generic int *var;
   return var;  
 }
 #ifndef CL20
-// expected-error@-5 {{OpenCL does not support the 'generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the 'generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the 'generic' type qualifier}}
+// expected-error@-5 {{OpenCL version 1.0 does not support the 'generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the 'generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the 'generic' type qualifier}}
 #endif
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -28,6 +28,7 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 using namespace clang;
 
@@ -3507,9 +3508,13 @@
   if (DiagID == diag::ext_duplicate_declspec)
 Diag(Tok, DiagID)
   << PrevSpec << FixItHint::CreateRemoval(Tok.getLocation());
-  else if (DiagID == diag::err_opencl_unknown_type_specifier)
-Diag(Tok, DiagID) << PrevSpec << isStorageClass;
-  else
+  else if (DiagID ==

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-07 Thread Vedran Miletić via cfe-commits 
rivanvx added inline comments.


Comment at: lib/Parse/ParseDecl.cpp:3519
@@ +3518,3 @@
+   / 100);
+const char *VerSpec = (VerMajor + std::string (".") + 
VerMinor).c_str();
+Diag(Tok, DiagID) << VerSpec << PrevSpec << isStorageClass;

Anastasia wrote:
> I think it will be nicer to use string (not char*) here too.
Other Spec are const char*, so I did it for consistency. But I don't care 
either way.


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-07 Thread Vedran Miletić via cfe-commits 
rivanvx updated this revision to Diff 56478.
rivanvx marked 3 inline comments as done.
rivanvx added a comment.

I am neither aware how to convert ints to StringRef nor how to concatenate 
StringRefs. Apologies if I missed something in the API.

In any case, this approach looks pretty clean to me.


http://reviews.llvm.org/D19780

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParseDecl.cpp
  test/Parser/opencl-cl20.cl
  test/Parser/opencl-storage-class.cl
  test/SemaOpenCL/invalid-access-qualifier.cl
  test/SemaOpenCL/storageclass.cl

Index: test/SemaOpenCL/storageclass.cl
===
--- test/SemaOpenCL/storageclass.cl
+++ test/SemaOpenCL/storageclass.cl
@@ -13,7 +13,7 @@
   constant int L1 = 0;
   local int L2;
 
-  auto int L3 = 7; // expected-error{{OpenCL does not support the 'auto' storage class specifier}}
+  auto int L3 = 7; // expected-error{{OpenCL version 1.2 does not support the 'auto' storage class specifier}}
   global int L4;   // expected-error{{function scope variable cannot be declared in global address space}}
 }
 
Index: test/SemaOpenCL/invalid-access-qualifier.cl
===
--- test/SemaOpenCL/invalid-access-qualifier.cl
+++ test/SemaOpenCL/invalid-access-qualifier.cl
@@ -10,5 +10,5 @@
 #ifdef CL20
 void test4(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'pipe int'}}
 #else
-void test4(__read_write image1d_t i) {} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' earlier than OpenCL2.0 version}}
+void test4(__read_write image1d_t i) {} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' earlier than OpenCL version 2.0}}
 #endif
Index: test/Parser/opencl-storage-class.cl
===
--- test/Parser/opencl-storage-class.cl
+++ test/Parser/opencl-storage-class.cl
@@ -2,10 +2,10 @@
 
 void test_storage_class_specs()
 {
-  static int a;// expected-error {{OpenCL does not support the 'static' storage class specifier}}
-  register int b;  // expected-error {{OpenCL does not support the 'register' storage class specifier}}
-  extern int c;// expected-error {{OpenCL does not support the 'extern' storage class specifier}}
-  auto int d;  // expected-error {{OpenCL does not support the 'auto' storage class specifier}}
+  static int a;// expected-error {{OpenCL version 1.0 does not support the 'static' storage class specifier}}
+  register int b;  // expected-error {{OpenCL version 1.0 does not support the 'register' storage class specifier}}
+  extern int c;// expected-error {{OpenCL version 1.0 does not support the 'extern' storage class specifier}}
+  auto int d;  // expected-error {{OpenCL version 1.0 does not support the 'auto' storage class specifier}}
 
 #pragma OPENCL EXTENSION cl_clang_storage_class_specifiers : enable
   static int e; // expected-error {{static local variable must reside in constant address space}}
Index: test/Parser/opencl-cl20.cl
===
--- test/Parser/opencl-cl20.cl
+++ test/Parser/opencl-cl20.cl
@@ -10,17 +10,17 @@
   return var;  
 }
 #ifndef CL20
-// expected-error@-5 {{OpenCL does not support the '__generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the '__generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the '__generic' type qualifier}}
+// expected-error@-5 {{OpenCL version 1.0 does not support the '__generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the '__generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the '__generic' type qualifier}}
 #endif
 
 generic int * generic_test(generic int *arg) {
   generic int *var;
   return var;  
 }
 #ifndef CL20
-// expected-error@-5 {{OpenCL does not support the 'generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the 'generic' type qualifier}}
-// expected-error@-6 {{OpenCL does not support the 'generic' type qualifier}}
+// expected-error@-5 {{OpenCL version 1.0 does not support the 'generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the 'generic' type qualifier}}
+// expected-error@-6 {{OpenCL version 1.0 does not support the 'generic' type qualifier}}
 #endif
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -28,6 +28,7 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/ScopedPrinter.h"
 
 using namespace clang;
 
@@ -3507,9 +3508,13 @@
   if (DiagID == diag::ext_duplicate_declspec)
 Diag(Tok, DiagID)
   << PrevSpec <<

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-07 Thread Vedran Miletić via cfe-commits 
rivanvx added a comment.

@Anastasia would you still prefer to make VerSpec a std::string?


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-06 Thread Anastasia Stulova via cfe-commits 
Anastasia added inline comments.


Comment at: lib/Parse/ParseDecl.cpp:3519
@@ +3518,3 @@
+   / 100);
+const char *VerSpec = (VerMajor + std::string (".") + 
VerMinor).c_str();
+Diag(Tok, DiagID) << VerSpec << PrevSpec << isStorageClass;

rivanvx wrote:
> Anastasia wrote:
> > I think it will be nicer to use string (not char*) here too.
> Other Spec are const char*, so I did it for consistency. But I don't care 
> either way.
Yes, it will be shorter. StringRef should work too I think.

Thanks!


http://reviews.llvm.org/D19780



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

Re: [PATCH] D19780: Output OpenCL version in Clang diagnostics

2016-05-06 Thread Anastasia Stulova via cfe-commits 
Anastasia added inline comments.


Comment at: lib/Parse/ParseDecl.cpp:3519
@@ +3518,3 @@
+   / 100);
+const char *VerSpec = (VerMajor + std::string (".") + 
VerMinor).c_str();
+Diag(Tok, DiagID) << VerSpec << PrevSpec << isStorageClass;

I think it will be nicer to use string (not char*) here too.


http://reviews.llvm.org/D19780



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