pxli168 added inline comments.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:4934
@@ +4933,3 @@
+ const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
+ if (AccessAttr->isReadWrite()) {
+ if (DeclTy->isPipeType() ||
----------------
Anastasia wrote:
> In the case of failure, where would destructor of AccessAttr be called?
>
> Could we invoke it before exiting? Or alternatively it's possible to check an
> access kind based on string matching from Attr getName() and create
> AccessAttr later on success.
Nice catch, I think I will try if getName() can handle this, or I think maybe I
need a destructor.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:5694
@@ +5693,3 @@
+ // attribute and may cause the same attribute to be added twice
+ if (const ParmVarDecl *PDecl = llvm::dyn_cast<ParmVarDecl>(D)) {
+ if (PDecl->getType().getCanonicalType().getTypePtr()->isPipeType())
----------------
Anastasia wrote:
> So where would the attribute check happen? I don't think the comment is very
> clear.
Maybe I don't make my self claer.
The pipe decl like
```
SomeFn(read_only pipe int p)
```
It will process read_only twice which is not expected, this will make attr add
twice.
I will make the comment clearer to understand.
================
Comment at: test/SemaOpenCL/invalid-kernel-attrs.cl:31
@@ -30,3 +30,3 @@
int __kernel x; // expected-error {{'__kernel' attribute only applies to
functions}}
- read_only int i; // expected-error {{'read_only' attribute only applies to
parameters}}
- __write_only int j; // expected-error {{'__write_only' attribute only
applies to parameters}}
+ read_only image1d_t i; // expected-error {{'read_only' attribute only
applies to parameters}}
+ __write_only image2d_t j; // expected-error {{'__write_only' attribute only
applies to parameters}}
----------------
Anastasia wrote:
> Strangely, I don't see this diagnostic being added in this patch.
As you can see, the read_only used to use with int. Now we check the type, so
int could not used for this test more. Just change the in to image1d_t to keep
the old test case work.
http://reviews.llvm.org/D16040
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
