[cfe-commits] [PATCH] Rejecting forbidden storage class specifiers in OpenCL

[Peter Collingbourne]

Hi George,

On Mon, Nov 29, 2010 at 11:15:26PM +0100, George Russell wrote:
> The OpenCL specification states that the storage class specifiers
> "extern","auto","static", and "register" are prohibited (in section
> 6.8.g, Restrictions).
> 
> This patch adds diagnostics for each of these, when the OpenCL
> language is selected, and a test case.
> 
> Feedback?

A few points:

The test should be moved to Sema (in DeclSpec::SetStorageClassSpec).
This will decrease code repetition, and Sema seems a more logical
place to perform the test.

We should forbid __private_extern__ also, I think.  While this storage
class is not mentioned in the OpenCL spec, it isn't in C99 either,
and doesn't make sense for OpenCL.

You should add the .cl extension to test/lit.cfg or the test case
will never run.  Also, our parser tests tend to use -fsyntax-only
(which causes only the parser and semantic analysis to run).

A single parametrised diagnostic would be better than one for each
storage class, as this decreases repetition as well as burden on
translators (if/when we get i18n support).

Less important: your code applies the storage-class specifier even
if it is prohibited, which doesn't seem right to me -- if we were
to downgrade this diagnostic to a warning, the resulting AST should
still be valid.

Thanks,
-- 
Peter