[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 18 22:16:03 PST 2018


rjmccall added a comment.

You're gating on OpenCL C++ in several places in this patch, but I don't think you need to.  This extension should be available to anyone using address spaces and C++.



================
Comment at: lib/Parse/ParseDecl.cpp:6162
+        }
+      }
+
----------------
This is enforcing a restriction that users write `const __private`, which seems unreasonable.  It looks like `ParseTypeQualifierList` takes a flag saying not to parse attributes; try adding a new option to that enum allowing address-space attributes.

Collecting the attributes on the type-qualifiers `DeclSpec` rather than adding them as function attributes seems correct.


================
Comment at: lib/Sema/SemaOverload.cpp:1157
+        NewMethod->getTypeQualifiers().getAddressSpace())
+      return true;
   }
----------------
If you leave `OldQuals` and `NewQuals` as `Qualifiers` and then just remove `restrict` from both sets, I think you don't need a special check for address spaces here.


================
Comment at: lib/Sema/SemaOverload.cpp:6671
+    }
+  }
 }
----------------
I would expect this to fail in `TryObjectArgumentInitialization` above and not to need a special case here.


================
Comment at: lib/Sema/SemaOverload.cpp:9279
+        (CandAS1 != LangAS::opencl_generic && CandAS1 != LangAS::Default))
+      return true;
+  }
----------------
This at least needs a comment explaining the rule you're trying to implement.


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

https://reviews.llvm.org/D55850





More information about the cfe-commits mailing list