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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D96816#2572589 <https://reviews.llvm.org/D96816#2572589>, @dexonsmith wrote:

> In D96816#2572552 <https://reviews.llvm.org/D96816#2572552>, @ahatanak wrote:
>
>> I still don't understand why the test was failing, but did we have to bump the version because a new LangOpt (`EncodeCXXClassTemplateSpec`) was added?
>
> Here are the RUN lines:
>
>   // RUN: mkdir -p %t/mod
>   // RUN: touch %t/empty.h
>   // RUN: cp %S/Inputs/preamble-reparse-changed-module/module.modulemap %t/mod
>   // RUN: cp %S/Inputs/preamble-reparse-changed-module/head.h %t/mod
>   
>   // RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_EXECUTE_COMMAND="cp %S/Inputs/preamble-reparse-changed-module/new-head.h %t/mod/head.h" CINDEXTEST_EXECUTE_AFTER_TRIAL=1 \
>   // RUN:     c-index-test -test-load-source-reparse 3 local %s -I %t -I %t/mod -fmodules -fmodules-cache-path=%t/mcp 2>&1 | FileCheck %s
>
> It wouldn't have failed in a clean build.
>
> The test is reusing the module cache path `%t/mcp` from the previous run. I think most implicit modules tests make themselves robust to incremental builds by adding something like this before the first use of the cache:
>
>   // RUN: rm -rf %t/mcp

Right; see also description of https://reviews.llvm.org/D73202

However, while that makes the test more robust, the test kind of found a real bug that actual users could've hit (...but only uses who build with LLVM_APPEND_VC_REV=NO), since users usually don't manually manage cache dirs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816