Hi, I have attached an updated patch addressing each of the issues mentioned.
Regards, George Russell On 6 December 2010 21:00, Peter Collingbourne <[email protected]> wrote: > 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 >
cl_storage_class_spec.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
