[PATCH, RFC] Fix PR16456 (make bool a context-sensitive keyword in Altivec mode)
Bill Schmidt
wschmidt at linux.vnet.ibm.com
Wed Jul 3 13:43:44 PDT 2013
On Wed, 2013-07-03 at 15:16 -0500, Bill Schmidt wrote:
> On Wed, 2013-07-03 at 13:08 -0700, Richard Smith wrote:
> > On Wed, Jul 3, 2013 at 12:32 PM, Bill Schmidt
> > <wschmidt at linux.vnet.ibm.com> wrote:
> > > "bool" should be a context-sensitive keyword in Altivec mode.
> > >
> > > PR16456 reported that Clang implements a hybrid between AltiVec's
> > > "Keyword and Predefine Method" and its "Context Sensitive Keyword
> > > Method," where "bool" is always a keyword, but "vector" and "pixel"
> > > are context-sensitive keywords. This isn't permitted by the AltiVec
> > > spec. For consistency with gcc, this patch implements the Context
> > > Sensitive Keyword Method for bool, and stops treating true and false
> > > as keywords in Altivec mode.
> > >
> > > The patch removes KEYALTIVEC as a trigger for defining these keywords
> > > in include/clang/Basic/TokenKinds.def, and adds logic for "vector
> > > bool" that mirrors the existing logic for "vector pixel." The test
> > > case is taken from the bug report.
> > >
> > > I know very little about the front end, so I would appreciate some
> > > review prior to committing the patch. Thanks!
> >
> > The patch contains windows line endings. Please make sure you don't
> > commit with windows line endings.
>
> That's...odd. Evolution must have messed it up. I work only in Linux.
>
> >
> > --- include/clang/Parse/Parser.h (revision 185433)
> > +++ include/clang/Parse/Parser.h (working copy)
> > @@ -531,7 +532,8 @@ class Parser : public CodeCompletionHandler {
> > bool &isInvalid) {
> > if (!getLangOpts().AltiVec ||
> > (Tok.getIdentifierInfo() != Ident_vector &&
> > - Tok.getIdentifierInfo() != Ident_pixel))
> > + Tok.getIdentifierInfo() != Ident_pixel &&
> > + Tok.getIdentifierInfo() != Ident_bool))
> >
> > This looks wrong:
> >
> > bool x;
> >
> > is not valid with -faltivec; bool is only a keyword after 'vector'. We
> > shouldn't be handling 'pixel' here either, FWIW. Per the AltiVec spec,
> > 2.2.2:
> >
> > "Since vector must be first among the type specifiers, it can be
> > recognized as a type
> > specifier when a type identifier is being scanned. The new uses of pixel
> > and bool occur
> > after vector has been recognized. In all other contexts, vector,
> > pixel, and bool are not
> > reserved."
>
> OK, I'll remove handling for both and re-test.
>
> >
> >
> > --- lib/Parse/ParseDecl.cpp (revision 185433)
> > +++ lib/Parse/ParseDecl.cpp (working copy)
> > @@ -5623,6 +5631,10 @@ bool Parser::TryAltiVecTokenOutOfLine(DeclSpec &DS
> > DS.isTypeAltiVecVector()) {
> > isInvalid = DS.SetTypeAltiVecPixel(true, Loc, PrevSpec, DiagID);
> > return true;
> > + } else if ((Tok.getIdentifierInfo() == Ident_bool) &&
> > + DS.isTypeAltiVecVector()) {
> > + isInvalid = DS.SetTypeAltiVecBool(true, Loc, PrevSpec, DiagID);
> > + return true;
> > }
> > return false;
> > }
> >
> > Likewise, this looks wrong, as does the handling of 'pixel' before it.
> >
>
> OK, likewise with the removal/re-test. ;)
Richard, if I remove either of these chunks, clang fails to recognize
"vector bool" and "vector pixel" as legitimate constructs.
Regarding the second chunk, I believe the test DS.isTypeAltiVecVector()
indicates that we've already recognized that we're inside a "vector"
decl when processing "pixel" or "bool". And without the first chunk we
never get to check things for the second.
Thanks,
Bill
>
> Thanks!
> Bill
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list