[PATCH] D146148: [clang] Add a namespace for interesting identifiers.

[Zahira Ammarguellat via Phabricator via cfe-commits]

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