[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-16 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert added a comment.

In D125429#3512642 , @aaronpuchert 
wrote:

> Proposed this in D125580 .

Landed this two days ago and no one complained about it so far, so let's hope 
that solved it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-13 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert added a comment.

In D125429#3511648 , @aaronpuchert 
wrote:

> And it appears that `unsigned long` does not have a clear favorite here.

Found it on StackOverflow: 
https://stackoverflow.com/questions/11603818/why-is-there-ambiguity-between-uint32-t-and-uint64-t-when-using-size-t-on-mac-os.

> Is that because `long` is 32-bit like on Windows, but at the same time it's 
> not the same as `unsigned` which is also 32-bit?

No. Mac is LP64 
, but uses 
`long long` for `(u)int64_t`.

> Perhaps we should overload on `(u)int32_t` instead `int`/`unsigned`?

Hence this doesn't solve it. I'm thinking that perhaps we should overload on 
`int`, `long`, `long long`. Seems silly, but C++ defines them as different 
types. Proposed this in D125580 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-13 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

https://github.com/llvm/llvm-project/commit/25862f53cce966cef2957825095861dec631d4f1
 fixed the build, thanks!

No opinion off the top of my head on changing the signature. Might be a good 
idea, maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-13 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert added a comment.

So we have overloads

  inline const StreamingDiagnostic <<(const StreamingDiagnostic , 
int I);
  inline const StreamingDiagnostic <<(const StreamingDiagnostic , 
int64_t I);
  inline const StreamingDiagnostic <<(const StreamingDiagnostic , 
unsigned I);
  inline const StreamingDiagnostic <<(const StreamingDiagnostic , 
uint64_t I);

And it appears that `unsigned long` does not have a clear favorite here. Is 
that because `long` is 32-bit like on Windows, but at the same time it's not 
the same as `unsigned` which is also 32-bit? Could we resolve this somehow so 
that we don't have the next person writing inconspicuous code to run into this?

Perhaps we should overload on `(u)int32_t` instead `int`/`unsigned`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-13 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert added a comment.

Hmm, didn't see this on Linux. I'll try to disambiguate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-13 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Hello, this broke building on mac:

- http://45.33.8.238/macm1/35247/step_4.txtp
- https://green.lab.llvm.org/green/job/clang-stage1-RA/

Please take a look, and revert for now if it takes a while to fix.

(Here's the error:

  In file included from ../../clang/lib/AST/CommentParser.cpp:9:
  In file included from ../../clang/include/clang/AST/CommentParser.h:16:
  In file included from ../../clang/include/clang/AST/Comment.h:17:
  In file included from ../../clang/include/clang/AST/DeclObjC.h:16:
  In file included from ../../clang/include/clang/AST/Decl.h:19:
  In file included from ../../clang/include/clang/AST/DeclBase.h:18:
  In file included from ../../clang/include/clang/AST/DeclarationName.h:16:
  In file included from ../../clang/include/clang/AST/Type.h:21:
  In file included from ../../clang/include/clang/AST/NestedNameSpecifier.h:18:
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: error: use of overloaded 
operator '<<' is ambiguous (with operand types 'const 
clang::StreamingDiagnostic' and 'typename remove_reference::type' (aka 'unsigned long'))
  DB << std::move(V);
  ~~ ^  
  ../../clang/lib/AST/CommentParser.cpp:417:57: note: in instantiation of 
function template specialization 'clang::DiagnosticBuilder::operator<<' requested here
  << CommandTok.is(tok::at_command) << Info->Name << Args.size()
  ^
  ../../clang/include/clang/Basic/Diagnostic.h:1400:35: note: candidate function
  inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
^
  ../../clang/include/clang/Basic/Diagnostic.h:1406:35: note: candidate function
  inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
^
  ../../clang/include/clang/Basic/Diagnostic.h:1422:35: note: candidate function
  inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
^
  ../../clang/include/clang/Basic/Diagnostic.h:1428:35: note: candidate function
  inline const StreamingDiagnostic <<(const StreamingDiagnostic ,
^
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, unsigned long)
  DB << std::move(V);
 ^
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, int)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, long long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, __int128)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, unsigned int)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, unsigned long long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(int, unsigned __int128)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(long, unsigned long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(long long, unsigned long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(__int128, unsigned long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(unsigned int, unsigned long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(unsigned long, unsigned long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(unsigned long long, unsigned long)
  ../../clang/include/clang/Basic/Diagnostic.h:1353:8: note: built-in candidate 
operator<<(unsigned __int128, unsigned long)

)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-13 Thread Aaron Puchert via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd3a4033d6ee1: Comment parsing: Allow inline commands to have 
0 or more than 1 argument (authored by aaronpuchert).

Changed prior to commit:
  https://reviews.llvm.org/D125429?vs=428968=429203#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

Files:
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/include/clang/AST/CommentParser.h
  clang/include/clang/AST/CommentSema.h
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/AST/ast-dump-comment.cpp
  clang/test/Headers/x86-intrinsics-headers-clean.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1116,49 +1116,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have a valid word argument}}
+// expected-warning@+1 {{'\a' command has no word arguments, expected 1}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+// expected-warning@+1 {{'\anchor' command has no word arguments, expected 1}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have a valid word argument}}
+// expected-warning@+1 {{'@b' command has no word arguments, expected 1}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have a valid word argument}}
+// expected-warning@+1 {{'\c' command has no word arguments, expected 1}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have a valid word argument}}
+// expected-warning@+1 {{'\e' command has no word arguments, expected 1}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have a valid word argument}}
+// expected-warning@+1 {{'\em' command has no word arguments, expected 1}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have a valid word argument}}
+// expected-warning@+1 {{'\p' command has no word arguments, expected 1}}
 /// \p
 int test_inline_no_argument_p_bad(int);
 
Index: clang/test/Headers/x86-intrinsics-headers-clean.cpp
===
--- clang/test/Headers/x86-intrinsics-headers-clean.cpp
+++ clang/test/Headers/x86-intrinsics-headers-clean.cpp
@@ -1,11 +1,11 @@
 // Make sure the intrinsic headers compile cleanly with no warnings or errors.
 
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // expected-no-diagnostics
Index: clang/test/AST/ast-dump-comment.cpp
===
--- clang/test/AST/ast-dump-comment.cpp
+++ clang/test/AST/ast-dump-comment.cpp
@@ -62,6 +62,12 @@
 // CHECK:  VarDecl{{.*}}Test_InlineCommandComment
 // CHECK:InlineCommandComment{{.*}} Name="c" RenderMonospaced Arg[0]="Aaa"
 
+/// \n Aaa
+int Test_InlineCommandComment_NoArgs;
+// CHECK:  VarDecl{{.*}}Test_InlineCommandComment_NoArgs
+// CHECK:InlineCommandComment{{.*}} Name="n" RenderNormal
+// CHECK-NEXT:   TextComment{{.*}} Text=" Aaa"
+
 /// \anchor Aaa
 int Test_InlineCommandCommentAnchor;
 // CHECK:  VarDecl{{.*}}Test_InlineCommandComment
Index: clang/lib/AST/CommentSema.cpp
===
---

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-12 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert updated this revision to Diff 428968.
aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

Unify `Argument` types and parsing. Add AST test. Simplify TableGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

Files:
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/include/clang/AST/CommentParser.h
  clang/include/clang/AST/CommentSema.h
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/AST/ast-dump-comment.cpp
  clang/test/Headers/x86-intrinsics-headers-clean.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1116,49 +1116,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have a valid word argument}}
+// expected-warning@+1 {{'\a' command has no word arguments, expected 1}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+// expected-warning@+1 {{'\anchor' command has no word arguments, expected 1}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have a valid word argument}}
+// expected-warning@+1 {{'@b' command has no word arguments, expected 1}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have a valid word argument}}
+// expected-warning@+1 {{'\c' command has no word arguments, expected 1}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have a valid word argument}}
+// expected-warning@+1 {{'\e' command has no word arguments, expected 1}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have a valid word argument}}
+// expected-warning@+1 {{'\em' command has no word arguments, expected 1}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have a valid word argument}}
+// expected-warning@+1 {{'\p' command has no word arguments, expected 1}}
 /// \p
 int test_inline_no_argument_p_bad(int);
 
Index: clang/test/Headers/x86-intrinsics-headers-clean.cpp
===
--- clang/test/Headers/x86-intrinsics-headers-clean.cpp
+++ clang/test/Headers/x86-intrinsics-headers-clean.cpp
@@ -1,11 +1,11 @@
 // Make sure the intrinsic headers compile cleanly with no warnings or errors.
 
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // expected-no-diagnostics
Index: clang/test/AST/ast-dump-comment.cpp
===
--- clang/test/AST/ast-dump-comment.cpp
+++ clang/test/AST/ast-dump-comment.cpp
@@ -62,6 +62,12 @@
 // CHECK:  VarDecl{{.*}}Test_InlineCommandComment
 // CHECK:InlineCommandComment{{.*}} Name="c" RenderMonospaced Arg[0]="Aaa"
 
+/// \n Aaa
+int Test_InlineCommandComment_NoArgs;
+// CHECK:  VarDecl{{.*}}Test_InlineCommandComment_NoArgs
+// CHECK:InlineCommandComment{{.*}} Name="n" RenderNormal
+// CHECK-NEXT: TextComment{{.*}} Text=" Aaa"
+
 /// \anchor Aaa
 int Test_InlineCommandCommentAnchor;
 // CHECK:  VarDecl{{.*}}Test_InlineCommandComment
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -265,10 +265,8 @@
 // User didn't

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-12 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert marked an inline comment as done.
aaronpuchert added inline comments.



Comment at: clang/include/clang/AST/CommentCommands.td:93
 
+def N  : InlineCommand<"n", 0>;
+

gribozavr2 wrote:
> Could you add a test that shows that the text after \n is not treated as the 
> argument?
> 
> You could verify the comment AST like here: 
> llvm-project/clang/test/AST/ast-dump-comment.cpp
> 
That's a good idea, I'll do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-12 Thread Simon Pilgrim via Phabricator via cfe-commits 
RKSimon added a comment.

Thanks @aaronpuchert !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-12 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/AST/CommentCommands.td:93
 
+def N  : InlineCommand<"n", 0>;
+

Could you add a test that shows that the text after \n is not treated as the 
argument?

You could verify the comment AST like here: 
llvm-project/clang/test/AST/ast-dump-comment.cpp



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-11 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert added inline comments.



Comment at: clang/include/clang/AST/Comment.h:303
-
-Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { }
   };

Removing that allows me to build an array without initializing all members 
right away. Alternative would be to add a default constructor here.



Comment at: clang/include/clang/Basic/DiagnosticCommentKinds.td:159
+def warn_doc_inline_command_not_enough_arguments : Warning<
+  "'%select{\\|@}0%1' command has %plural{0:no|:%2}2 word argument%s2, 
expected %3">,
   InGroup, DefaultIgnore;

If you find that `%plural` too fancy, a plain `%2` should also do.



Comment at: clang/lib/AST/CommentParser.cpp:295-307
   typedef BlockCommandComment::Argument Argument;
   Argument *Args =
   new (Allocator.Allocate(NumArgs)) Argument[NumArgs];
   unsigned ParsedArgs = 0;
   Token Arg;
   while (ParsedArgs < NumArgs && Retokenizer.lexWord(Arg)) {
 Args[ParsedArgs] = Argument(SourceRange(Arg.getLocation(),

This is basically what I'm duplicating. As it happens, the two `Argument` 
structures are structurally the same, so we could unify them and factor out 
this code. I'd probably do that in a separate change though (prior to this or 
as follow-up).



Comment at: clang/test/Headers/x86-intrinsics-headers-clean.cpp:4
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual 
-Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual 
-Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers 
-flax-vector-conversions=none -x c++ -verify %s

@RKSimon, according to https://github.com/llvm/llvm-project/issues/35297 you 
mostly wanted -pedantic, but I took the liberty of enabling both. They're 
currently coupled (similar to 
https://github.com/llvm/llvm-project/issues/19442).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

2022-05-11 Thread Aaron Puchert via Phabricator via cfe-commits 
aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
Herald added a project: All.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

That's required to support `\n`, but can also used for other commands.
The argument parsing is basically duplicated from block commands, but we
can't easily factor that out as they have a different argument data
type.

This should fix #55319.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125429

Files:
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/include/clang/AST/CommentSema.h
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/Headers/x86-intrinsics-headers-clean.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1116,49 +1116,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have a valid word argument}}
+// expected-warning@+1 {{'\a' command has no word arguments, expected 1}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+// expected-warning@+1 {{'\anchor' command has no word arguments, expected 1}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have a valid word argument}}
+// expected-warning@+1 {{'@b' command has no word arguments, expected 1}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have a valid word argument}}
+// expected-warning@+1 {{'\c' command has no word arguments, expected 1}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have a valid word argument}}
+// expected-warning@+1 {{'\e' command has no word arguments, expected 1}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have a valid word argument}}
+// expected-warning@+1 {{'\em' command has no word arguments, expected 1}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have a valid word argument}}
+// expected-warning@+1 {{'\p' command has no word arguments, expected 1}}
 /// \p
 int test_inline_no_argument_p_bad(int);
 
Index: clang/test/Headers/x86-intrinsics-headers-clean.cpp
===
--- clang/test/Headers/x86-intrinsics-headers-clean.cpp
+++ clang/test/Headers/x86-intrinsics-headers-clean.cpp
@@ -1,11 +1,11 @@
 // Make sure the intrinsic headers compile cleanly with no warnings or errors.
 
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // expected-no-diagnostics
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -361,37 +361,15 @@
   checkBlockCommandEmptyParagraph(Command);
 }
 
-InlineCommandComment *Sema::actOnInlineCommand(SourceLocation CommandLocBegin,
-   SourceLocation CommandLocEnd,
-   unsigned CommandID) {
-  ArrayRef Args;
-  StringRef CommandName = Traits.getCommandInfo(CommandID)->Name;
-  return new (Allocator) InlineCommandComment(
-  CommandLocBegin,
-  CommandLocEnd,
-  CommandID,
-