zahiraam added inline comments.
================
Comment at: clang/include/clang/Basic/IdentifierTable.h:325
void setBuiltinID(unsigned ID) {
- ObjCOrBuiltinID = ID + tok::NUM_OBJC_KEYWORDS;
- assert(ObjCOrBuiltinID - unsigned(tok::NUM_OBJC_KEYWORDS) == ID
- && "ID too large for field!");
+ ObjCOrBuiltinID = FirstBuiltinID + (ID - 1);
+ assert(getBuiltinID() == ID && "ID too large for field!");
----------------
rjmccall wrote:
> `initializeBuiltins` does call `setBuiltinID(NotBuiltin)` in order to reset
> any identifiers for which we have a `-fno-builtin=X` argument, so you need
> to handle that somehow, because otherwise this is going to underflow and set
> up the identifier with the last `InterestingIdentifierKind`. There are two
> options:
> - You could just make `initializeBuiltins` call some sort of `clearBuiltinID`
> method that resets `ObjCOrBuiltinID` to 0. That has the advantage of making
> the "lifecycle" of this field much more straightforward. For example, we
> probably ought to be asserting in these methods that we're not overwriting
> existing data; `clearBuiltinID` would know that it was okay to clear data,
> but only from a builtin.
> - You could also just check for `NotBuiltin` and set `ObjCBuiltinID = 0` when
> you see it.
> `initializeBuiltins` does call `setBuiltinID(NotBuiltin)` in order to reset
> any identifiers for which we have a `-fno-builtin=X` argument, so you need
> to handle that somehow, because otherwise this is going to underflow and set
> up the identifier with the last `InterestingIdentifierKind`. There are two
> options:
> - You could just make `initializeBuiltins` call some sort of `clearBuiltinID`
> method that resets `ObjCOrBuiltinID` to 0. That has the advantage of making
> the "lifecycle" of this field much more straightforward. For example, we
> probably ought to be asserting in these methods that we're not overwriting
> existing data; `clearBuiltinID` would know that it was okay to clear data,
> but only from a builtin.
Would that be an additional step in `initializeBuiltins` or part of step #4? I
understants intuitively what the issue would be here if we didn't do that, but
I can't find a test case that goes through that step. Would be clearer to me.
> - You could also just check for `NotBuiltin` and set `ObjCBuiltinID = 0` when
> you see it.
I don't think this can be done here. builtin::NotBuiltin is not known here in
IdentierTable.h (get a lots of build errors). So, this needs to be done in
`initializerBultins` method.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146148/new/
https://reviews.llvm.org/D146148
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
