zahiraam marked an inline comment as done. zahiraam added inline comments.
================ Comment at: clang/lib/Sema/SemaDecl.cpp:6784 + if (II->getInterestingIdentifierID() != 0) + NewTD->addAttr(AvailableOnlyInDefaultEvalMethodAttr::Create(Context)); } ---------------- rjmccall wrote: > zahiraam wrote: > > rjmccall wrote: > > > Please switch over the interesting identifiers here; we don't want to > > > assume this feature is only used for these two names. > > > > > > In fact, should we go ahead and immediately apply it to the four > > > identifiers above this? That would be nice, because then we could > > > actually do this in two patches: one patch that does the refactor to > > > track interesting identifiers but doesn't cause any functionality changes > > > and a second, very small patch that just introduces the new special > > > treatment for `float_t` and `double_t`. > > > Please switch over the interesting identifiers here; we don't want to > > > assume this feature is only used for these two names. > > > > > > In fact, should we go ahead and immediately apply it to the four > > > identifiers above this? That would be nice, because then we could > > > actually do this in two patches: one patch that does the refactor to > > > track interesting identifiers but doesn't cause any functionality changes > > > and a second, very small patch that just introduces the new special > > > treatment for `float_t` and `double_t`. > > > > Are you saying that "FILE", "jmp_buf"," sigjmp_buf" and "ucontext_t" are > > also interesting identifiers? If yes, they should be added to the list of > > interesting identifiers in TokenKinds.def? > Right. The basic idea of interesting identifiers is to replace these sorts > of identifier comparisons in performance-critical code. So your first patch > would *only* add those four identifiers as interesting identifiers, handling > them here by registering the `typedef` with the ASTContext like the code is > already doing. Then you'd make a follow-up patch that adds `float_t` and > `double_t` and handles them here by implicitly adding your new attribute. > Right. The basic idea of interesting identifiers is to replace these sorts > of identifier comparisons in performance-critical code. So your first patch > would *only* add those four identifiers as interesting identifiers, handling > them here by registering the `typedef` with the ASTContext like the code is > already doing. Then you'd make a follow-up patch that adds `float_t` and > `double_t` and handles them here by implicitly adding your new attribute. I think that does it? 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