[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I managed to reproduce the failure by building everything with `-DLLVM_APPEND_VC_REV=NO`.

What I found was that any `BENIGN_LANGOPT` added to `LangOptions.def` would cause the test to fail since `CompilerInvocation::getModuleHash` doesn't use the option to compute the hash if it's a `BENIGN_LANGOPT`. The test was added a month ago, so I think my commit was the first to cause the failure. If I use `LANGOPT` instead of `BENIGN_LANGOPT`, the test passes.

The description of `BENIGN_LANGOPT` says "for options that don't affect the construction of the AST in any way" and since the option can change the AST (e.g., the value of `unsigned length = sizeof(@encode(S));` will differ depending on whether the member for the option is set or not), I think I should have used `LANGOPT` instead of `BENIGN_LANGOPT`.

Since this is something that is easy to overlook, should there be a comment to remind people to update the version whenever they add a `BENIGN_LANGOPT`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816