[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGacbb3652931a: [AST][NFC] Silence GCC warning about multiline 
comments (authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

Files:
  clang/include/clang/AST/DeclOpenMP.h


Index: clang/include/clang/AST/DeclOpenMP.h
===
--- clang/include/clang/AST/DeclOpenMP.h
+++ clang/include/clang/AST/DeclOpenMP.h
@@ -163,7 +163,7 @@
 /// 'float':
 ///
 /// \code
-/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
+/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in)
 /// initializer (omp_priv = 0)
 /// \endcode
 ///


Index: clang/include/clang/AST/DeclOpenMP.h
===
--- clang/include/clang/AST/DeclOpenMP.h
+++ clang/include/clang/AST/DeclOpenMP.h
@@ -163,7 +163,7 @@
 /// 'float':
 ///
 /// \code
-/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
+/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in)
 /// initializer (omp_priv = 0)
 /// \endcode
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2481300 , @rjmccall wrote:

> People can't actually just copy/paste from the comment anyway because of the 
> comment characters on the line, and I don't think anyone will misunderstand 
> the example if the backslash is missing.  It's silly that GCC has a problem 
> with this, but since it does, let's just work around the problem and drop the 
> backslash rather than having one comment block in the header use a different 
> style.

They can from the online documentation 
(https://clang.llvm.org/doxygen/classclang_1_1OMPDeclareReductionDecl.html#details)
 but since other approaches are unfortunately not working this is probably the 
best approach indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre updated this revision to Diff 315080.
thopre added a comment.

Change approach to remove continuation line instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

Files:
  clang/include/clang/AST/DeclOpenMP.h


Index: clang/include/clang/AST/DeclOpenMP.h
===
--- clang/include/clang/AST/DeclOpenMP.h
+++ clang/include/clang/AST/DeclOpenMP.h
@@ -163,7 +163,7 @@
 /// 'float':
 ///
 /// \code
-/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
+/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in)
 /// initializer (omp_priv = 0)
 /// \endcode
 ///


Index: clang/include/clang/AST/DeclOpenMP.h
===
--- clang/include/clang/AST/DeclOpenMP.h
+++ clang/include/clang/AST/DeclOpenMP.h
@@ -163,7 +163,7 @@
 /// 'float':
 ///
 /// \code
-/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
+/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in)
 /// initializer (omp_priv = 0)
 /// \endcode
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

People can't actually just copy/paste from the comment anyway because of the 
comment characters on the line, and I don't think anyone will misunderstand the 
example if the backslash is missing.  It's silly that GCC has a problem with 
this, but since it does, let's just work around the problem and drop the 
backslash rather than having one comment block in the header use a different 
style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2450897 , @thopre wrote:

> In D92409#2450690 , @rsmith wrote:
>
>> In D92409#2450550 , @thopre wrote:
>>
>>> In D92409#2426196 , @thopre wrote:
>>>
 Is there a way to disable it from the header? I've noticed this warning 
 when compiling an application using libclang with g++. So I'm looking for 
 a fix outside clang's build system which is not used in that case.
>>>
>>> Ping?
>>
>> `#pragma GCC diagnostic ignored` 
>> (https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html) can do it. Now, 
>> we can't reasonably guarantee that Clang headers won't cause arbitrary 
>> broken warnings to fire, so there's a judgment call here on the extent to 
>> which we should carry changes to support use of such warning flags (versus 
>> expecting the code including the header to disable the warning). In this 
>> case, it's probably worth it, though, because the warning in question is 
>> part of GCC's `-Wall`.
>
> The documentation for that pragma says:
>
>> Note that not all diagnostics are modifiable; at the moment only warnings 
>> (normally controlled by ‘-W…’) can be controlled, and not all of them.
>
> Unfortunately -Wcomment seems to be one of them as the pragma has no effect 
> on the warning, at least with GCC 7, I haven't tried other versions.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2450690 , @rsmith wrote:

> In D92409#2450550 , @thopre wrote:
>
>> In D92409#2426196 , @thopre wrote:
>>
>>> Is there a way to disable it from the header? I've noticed this warning 
>>> when compiling an application using libclang with g++. So I'm looking for a 
>>> fix outside clang's build system which is not used in that case.
>>
>> Ping?
>
> `#pragma GCC diagnostic ignored` 
> (https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html) can do it. Now, 
> we can't reasonably guarantee that Clang headers won't cause arbitrary broken 
> warnings to fire, so there's a judgment call here on the extent to which we 
> should carry changes to support use of such warning flags (versus expecting 
> the code including the header to disable the warning). In this case, it's 
> probably worth it, though, because the warning in question is part of GCC's 
> `-Wall`.

The documentation for that pragma says:

> Note that not all diagnostics are modifiable; at the moment only warnings 
> (normally controlled by ‘-W…’) can be controlled, and not all of them.

Unfortunately -Wcomment seems to be one of them as the pragma has no effect on 
the warning, at least with GCC 7, I haven't tried other versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D92409#2450550 , @thopre wrote:

> In D92409#2426196 , @thopre wrote:
>
>> Is there a way to disable it from the header? I've noticed this warning when 
>> compiling an application using libclang with g++. So I'm looking for a fix 
>> outside clang's build system which is not used in that case.
>
> Ping?

`#pragma GCC diagnostic ignored` 
(https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html) can do it. Now, we 
can't reasonably guarantee that Clang headers won't cause arbitrary broken 
warnings to fire, so there's a judgment call here on the extent to which we 
should carry changes to support use of such warning flags (versus expecting the 
code including the header to disable the warning). In this case, it's probably 
worth it, though, because the warning in question is part of GCC's `-Wall`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2426196 , @thopre wrote:

> In D92409#2426188 , @rsmith wrote:
>
>> Let's just disable this broken GCC warning. IIRC, the corresponding clang 
>> warning covers the actual bugs caused by line continuation in comments, and 
>> doesn't warn if the next line begins with a `//`.
>
> Is there a way to disable it from the header? I've noticed this warning when 
> compiling an application using libclang with g++. So I'm looking for a fix 
> outside clang's build system which is not used in that case.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2426188 , @rsmith wrote:

> Let's just disable this broken GCC warning. IIRC, the corresponding clang 
> warning covers the actual bugs caused by line continuation in comments, and 
> doesn't warn if the next line begins with a `//`.

Is there a way to disable it from the header? I've noticed this warning when 
compiling an application using libclang with g++. So I'm looking for a fix 
outside clang's build system which is not used in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Let's just disable this broken GCC warning. IIRC, the corresponding clang 
warning covers the actual bugs caused by line continuation in comments, and 
doesn't warn if the next line begins with a `//`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

In D92409#2426160 , @ABataev wrote:

> I would just remove this extra symbol.

But then the example code is no longer copy/pastable. Note that I expect people 
to copy/paste but it's nice if it looks authentic. Note that I don't mind 
either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I would just remove this extra symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre created this revision.
thopre added reviewers: ABataev, rjmccall, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
thopre requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

GCC's -Wcomment warning is emitted on OMPDeclareReductionDecl heading
comment due to the continuation mark in the code example. This commit
changes the comment style to javadoc to avoid the warning and allow
building with -Werror.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92409

Files:
  clang/include/clang/AST/DeclOpenMP.h


Index: clang/include/clang/AST/DeclOpenMP.h
===
--- clang/include/clang/AST/DeclOpenMP.h
+++ clang/include/clang/AST/DeclOpenMP.h
@@ -158,16 +158,18 @@
   static bool classofKind(Kind K) { return K == OMPThreadPrivate; }
 };
 
-/// This represents '#pragma omp declare reduction ...' directive.
-/// For example, in the following, declared reduction 'foo' for types 'int' and
-/// 'float':
-///
-/// \code
-/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
-/// initializer (omp_priv = 0)
-/// \endcode
-///
-/// Here 'omp_out += omp_in' is a combiner and 'omp_priv = 0' is an 
initializer.
+/**
+ * This represents '#pragma omp declare reduction ...' directive.
+ * For example, in the following, declared reduction 'foo' for types 'int' and
+ * 'float':
+ *
+ * \code
+ * #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
+ * initializer (omp_priv = 0)
+ * \endcode
+ *
+ * Here 'omp_out += omp_in' is a combiner and 'omp_priv = 0' is an initializer.
+ */
 class OMPDeclareReductionDecl final : public ValueDecl, public DeclContext {
   // This class stores some data in DeclContext::OMPDeclareReductionDeclBits
   // to save some space. Use the provided accessors to access it.


Index: clang/include/clang/AST/DeclOpenMP.h
===
--- clang/include/clang/AST/DeclOpenMP.h
+++ clang/include/clang/AST/DeclOpenMP.h
@@ -158,16 +158,18 @@
   static bool classofKind(Kind K) { return K == OMPThreadPrivate; }
 };
 
-/// This represents '#pragma omp declare reduction ...' directive.
-/// For example, in the following, declared reduction 'foo' for types 'int' and
-/// 'float':
-///
-/// \code
-/// #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
-/// initializer (omp_priv = 0)
-/// \endcode
-///
-/// Here 'omp_out += omp_in' is a combiner and 'omp_priv = 0' is an initializer.
+/**
+ * This represents '#pragma omp declare reduction ...' directive.
+ * For example, in the following, declared reduction 'foo' for types 'int' and
+ * 'float':
+ *
+ * \code
+ * #pragma omp declare reduction (foo : int,float : omp_out += omp_in) \
+ * initializer (omp_priv = 0)
+ * \endcode
+ *
+ * Here 'omp_out += omp_in' is a combiner and 'omp_priv = 0' is an initializer.
+ */
 class OMPDeclareReductionDecl final : public ValueDecl, public DeclContext {
   // This class stores some data in DeclContext::OMPDeclareReductionDeclBits
   // to save some space. Use the provided accessors to access it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits