[LLVMdev] [PATCH] OpenCL support - update on keywords

[Anton Lokhmotov]

Hi Peter,

Many thanks for your prompt review.

> I don't think we should include the word OpenCL in the name of this
> enum/header.  We may very well in the future want to define address
> space constants for other languages, which should not overlap with
> the OpenCL constants, by extending this enum.
Good point.  We have defined in llvm/Support/AddressSpaces.h: 

+namespace llvm {
+  namespace AddressSpaces {
+    enum AddressSpace {
+      PRIVATE = 0,
+      GLOBAL = 1,
+      LOCAL = 2,
+      CONSTANT = 3
+    };
+  }
+}

(We were not quite sure whether to keep OPENCL_PRIVATE, etc., so removed the
prefix as well.  Feel free to put it back if needed.)

> "private" should not be marked as a type qualifier, type specifier
> qualifier or declaration specifier in languages other than OpenCL.
Fully agree.  Should be fixed now.

> There are several 80-col violations in lib/Parse/ParseDecl.cpp
> and in the CreateIntegerAttribute function.  git also picked
> up some trailing whitespace errors.
Should be fixed now.

> To avoid the code duplication between ParseDeclarationSpecifiers,
> ParseOptionalTypeSpecifier and ParseTypeQualifierListOpt you
> should introduce a ParseOpenCLTypeAttributes function (e.g. see
> ParseBorlandTypeAttributes).
Done.

> Other than that, LGTM.

Thanks,
Anton.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 00002-keywords-llvm.patch
Type: application/octet-stream
Size: 935 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110224/92f609a1/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 00002-keywords-clang.patch
Type: application/octet-stream
Size: 15628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110224/92f609a1/attachment-0001.obj>