egorzhdan added inline comments.
================
Comment at: clang/test/Index/comment-lots-of-unknown-commands.c:3
+// XFAIL: *
+
----------------
aaronpuchert wrote:
> egorzhdan wrote:
> > gribozavr2 wrote:
> > > egorzhdan wrote:
> > > > This test was never properly passing. Because of the bug in string
> > > > conversion, the printed comments contained the entire source file and
> > > > not just the comments' text, which was enough to cause `// CHECK`-s in
> > > > the test to succeed.
> > > > ```
> > > > // CHECK: (CXComment_InlineCommand CommandName=[tel]
> > > > RenderNormal HasTrailingNewline)
> > > > // CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal
> > > > HasTrailingNewline))
> > > > // CHECK: (CXComment_VerbatimLine
> > > > Text=[\n@Lo\n@il\n@tle\n@axt\n@ba\n@ust\n@ac\n@tpe\n@tpl\n@ctG\n@ru\n@m\n@tG\n@it\n@rh\n@G\n@rpc\n@el\n@er\n@w\n@eo\n@tx\n@oo\n@dD\n@dD\n*/\nvoid
> > > > f();\n\n// CHECK: CommentAST=[\n// CHECK:
> > > > (CXComment_FullComment\n// CHECK: (CXComment_Paragraph\n// CHECK:
> > > > ...
> > > > ```
> > > Please update the test to pass then. Here's the diff:
> > >
> > > ```
> > > diff --git a/clang/test/Index/comment-lots-of-unknown-commands.c
> > > b/clang/test/Index/comment-lots-of-unknown-commands.c
> > > index 41a03d394488..e1adcc150b1e 100644
> > > --- a/clang/test/Index/comment-lots-of-unknown-commands.c
> > > +++ b/clang/test/Index/comment-lots-of-unknown-commands.c
> > > @@ -1,6 +1,5 @@
> > > // RUN: c-index-test -test-load-source-reparse 1 local %s | FileCheck %s
> > >
> > > -// XFAIL: *
> > >
> > > // See PR 21254. We had too few bits to encode command IDs so if you
> > > created
> > > // enough of them the ID codes would wrap around. This test creates
> > > commands up
> > > @@ -183,7 +182,7 @@ void f();
> > > // CHECK: (CXComment_InlineCommand CommandName=[ei] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[oun]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[ou] RenderNormal
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[nl] RenderNormal
> > > HasTrailingNewline)
> > > +// CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[ien]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[fr] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[en] RenderNormal
> > > HasTrailingNewline)
> > > @@ -204,7 +203,7 @@ void f();
> > > // CHECK: (CXComment_InlineCommand CommandName=[fro]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[ast]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[ae] RenderNormal
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[nN] RenderNormal
> > > HasTrailingNewline)
> > > +// CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[pc] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[tae]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[ws] RenderNormal
> > > HasTrailingNewline)
> > > @@ -268,10 +267,8 @@ void f();
> > > // CHECK: (CXComment_InlineCommand CommandName=[an] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[de] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[tel]
> > > RenderNormal HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[nd] RenderNormal
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[dic]
> > > RenderNormal HasTrailingNewline)
> > > +// CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[Lo] RenderNormal
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[il] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[tle]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[axt]
> > > RenderNormal HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[ba] RenderNormal
> > > HasTrailingNewline)
> > > @@ -283,7 +280,6 @@ void f();
> > > // CHECK: (CXComment_InlineCommand CommandName=[ru] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[m] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[tG] RenderNormal
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[it] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[rh] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[G] RenderNormal
> > > HasTrailingNewline)
> > > // CHECK: (CXComment_InlineCommand CommandName=[rpc]
> > > RenderNormal HasTrailingNewline)
> > > ```
> > >
> > This doesn't look like the correct output though. I think the `CommandName`
> > fields in the output should match the `@`-tags in the input (e.g. `@s` ->
> > `(CXComment_InlineCommand CommandName=[s] RenderNormal
> > HasTrailingNewline)`).
> >
> > This looks like a bug in the logic this test is aiming to verify, not a
> > mistake in the test itself. I unfortunately don't have enough context to
> > fix the actual issue here, so I disabled the test instead. I'll also file
> > an GitHub issue for this.
> They don't always match, because typo correction will change commands that
> are close enough to an existing command. That's why e.g. `@nl` shows up as
> `@n` in the AST. I remember that I had to change this test in D111190 because
> introducing the new commands made the typo correction kick in for some of
> these.
>
> The proper fix for the test (see the comment below) should be to change the
> commands here to differ enough from existing commands again. The corrections
> to `@n` are likely caused by D125429, while `@dic` → `@dir`, `@il`/`@it` →
> `@if` come from D111190.
>
> I don't think we're going to gain additional commands, but we might also make
> this "future-proof" by simply using long-enough sequences of random letters,
> let's say 4 to have a reasonable number.
Thanks @aaronpuchert! I didn't realize this is caused by typo correction. I'll
put up a patch to re-enable the test.
================
Comment at: clang/tools/libclang/CXString.cpp:81-82
CXString createRef(StringRef String) {
+ if (!String.data())
+ return createNull();
+
----------------
aaronpuchert wrote:
> Does this actually happen or are we just doing this to be safe?
It's possible to get an empty StringRef with a nullptr data here. I initially
skipped this `if` and got a test failure because of it: instead of returning a
null CXString, this method was returning an empty CXString.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133009/new/
https://reviews.llvm.org/D133009
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
