[PATCH] D93095: Introduce -Wreserved-identifier

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.


================
Comment at: clang/lib/AST/Decl.cpp:1097
+  // ignored values) that we don't warn on it.
+  if (Name.size() <= 1)
+    return ReservedIdentifierStatus::NotReserved;
----------------
rsmith wrote:
> Would it make sense to move the rest of this function to a member on `IdentifierInfo`? That is, the rest of this function would become:
> 
> ```
> ReservedIdentifierStatus Status = II->isReserved(LangOpts);
> if (Status == ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope &&
>     isa<ParmVarDecl>(this) || isTemplateParameter() ||
>     !getDeclContext()->getRedeclContext()->isTranslationUnit())
>   return ReservedIdentifierStatus::NotReserved;
> return Status;
> ```
> 
> That way, we should be able to reuse the same implementation to detect reserved macros too.
I did the `II->isReserved(LangOpts)` part, but the updated condition `!getDeclContext()->getRedeclContext()->isTranslationUnit()` fails the validation. I did already spend a lot of time getting one that catches the behavior we wanted, so 'm not super-eager to investigate why this change would not work ;-)


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

https://reviews.llvm.org/D93095