[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:16:49 PDT 2013


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. ;)

Thanks!  
Bill




More information about the cfe-commits mailing list