Manna added inline comments.
================
Comment at: clang/lib/Parse/ParsePragma.cpp:4058
 
+  PP.Lex(Tok);
+  II = Tok.getIdentifierInfo();
----------------
erichkeane wrote:
> I'm having trouble figuring out why this isn't a breaking change.  
> `PP.Lex(Tok)` moves the current 'token' to be the next one (that is, it has 
> side effects!), so now this ends up looking in a different place for the 
> intrin name? 
> 
> This will result in `IntrinsicClass` becoming the value of the token 2 after 
> the fact, which doesn't seem right to me.  I suspect the correct answer here 
> is to just move IntrinsicClass ~4050.
I am wondering whether do we need IntrinsicClass`. Can we do something like:
```
if (II->isStr("vector"))
    Actions.DeclareRISCVVBuiltins = true;
  else if (II->isStr("sifive_vector")
    Actions.DeclareRISCVVectorBuiltins = true;
```


================
Comment at: clang/lib/Parse/ParsePragma.cpp:4042
 
   PP.Lex(Tok);
   if (!II || !(II->isStr("vector") || II->isStr("sifive_vector"))) {
----------------
erichkeane wrote:
> I think you still need line 4043 here, right?  The one where II is set to the 
> Token?
oops! Fixed now. Thanks @erichkeane for catching!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150895/new/

https://reviews.llvm.org/D150895

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to