sammccall added a comment. In D134813#3834776 <https://reviews.llvm.org/D134813#3834776>, @aaron.ballman wrote:
> Thank you for this! I applied D135191 <https://reviews.llvm.org/D135191> over > the top of my changes here and ran the tests to get all the new failures with > the changes, then I reverted those tests which failed back to their trunk > form. When I re-ran the tests, I get the following failures: > > ******************** TEST 'Clang :: Index/usrs.m' FAILED > ******************** > ... > $ "f:\source\llvm-project\llvm\out\build\x64-debug\bin\filecheck.exe" > "-check-prefix=CHECK-source" "F:\source\llvm-project\clang\test\Index\usrs.m" > # command stderr: > F:\source\llvm-project\clang\test\Index\usrs.m:188:18: error: CHECK-source: > expected string not found in input > // CHECK-source: usrs.m:5:1: EnumDecl=:5:1 (Definition) Extent=[5:1 - 8:2] > ^ > <stdin>:386:63: note: scanning from here > // CHECK: usrs.m:3:56: DeclRefExpr=y:3:40 Extent=[3:56 - 3:57] > ^ > <stdin>:387:89: note: possible intended match here > // CHECK: usrs.m:5:1: EnumDecl=enum (unnamed at > F:\source\llvm-project\clang\test\Index\usrs.m:5:1):5:1 (Definition) > Extent=[5:1 - 8:2] This isn't a change to USRs, it's a change to libclang's `clang_getCursorSpelling`. I think it's safe to update (unlike the USRs earlier in the file). Since this is a general-purpose API it makes sense that it would follow the change in printName() defaults. > ******************** TEST 'Clang :: ExtractAPI/enum.c' FAILED > ******************** > # command output: > *** > F:\source\llvm-project\llvm\out\build\x64-Debug\tools\clang\test\ExtractAPI\Output\enum.c.tmp/reference.output.json > --- > F:\source\llvm-project\llvm\out\build\x64-Debug\tools\clang\test\ExtractAPI\Output\enum.c.tmp/output-normalized.json > *************** > *** 655,667 **** > "navigator": [ > { > "kind": "identifier", > ! "spelling": "(anonymous)" > ! } > ! ], > ! "title": "(anonymous)" > ! }, > ! "pathComponents": [ > ! "(anonymous)" > ] > }, > { > --- 655,667 ---- > "navigator": [ > { > "kind": "identifier", > ! "spelling": "enum (unnamed at > F:\\source\\llvm-project\\llvm\\out\\build\\x64-Debug\\tools\\clang\\test\\ExtractAPI\\Output\\enum.c.tmp/input.h:17:1)" > ! } > ! ], > ! "title": "enum (unnamed at > F:\\source\\llvm-project\\llvm\\out\\build\\x64-Debug\\tools\\clang\\test\\ExtractAPI\\Output\\enum.c.tmp/input.h:17:1)" > ! }, > ! "pathComponents": [ > ! "enum (unnamed at > F:\\source\\llvm-project\\llvm\\out\\build\\x64-Debug\\tools\\clang\\test\\ExtractAPI\\Output\\enum.c.tmp/input.h:17:1)" > ] > }, > ... ExtractAPI seems to produce a data description of the AST for programmatic consumption (apple swift interop?), this looks like a breaking change to me. It looks like they have at least one explicit check similar to USRs in clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:361, but I know very little about how this tool is used out-of-tree by Apple: I'm not sure how much the exact strings/lack of strings matters, may need owners of that tool involved here. > ******************** TEST 'Clang :: Index/annotate-comments-typedef.m' > FAILED ******************** I can't see the actual output of the tool in that log, but I guess this is related to clang/lib/Index/CommentToXML.cpp:908. I don't know if reporting exactly `<anonymous>` is important behavior that should be preserved, or `(unnamed)` would be fine. I don't even know what CommentToXML is: again I guess it's consumed by something related to XCode out-of-tree. > and also the clangd unit tests failed: > > [----------] 1 test from FindExplicitReferencesTest > [ RUN ] FindExplicitReferencesTest.All > ... > With diff: > @@ -1,3 +1,3 @@ > -0: targets = {} > +0: targets = {(unnamed)} > 1: targets = {x}, decl > 2: targets = {fptr}, decl > > > void foo() { > $0^class {} $1^x; > int (*$2^fptr)(int $3^a, int) = nullptr; > } Yes, this is expected: the test identifies which target is referenced by name, and we changed the name from "" to "(unnamed)". This patch can change the test data on clang-tools-extra/clangd/unittests/FindTargetTests.cpp:1619 from `targets = {}` to `targets={(unnamed)}`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134813/new/ https://reviews.llvm.org/D134813 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
