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

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

Reply via email to