Re: [PATCH v2] c: Silently ignore pragma region [PR85487]
On the contrary, as a user of GCC I would much prefer a consistent behavior for #pragma region based purely on GCC version. IE, so you can tell people: "just update to GCC X.Y and those warnings will go away" rather than: "update to GCC X.Y and pass some new flags - but make sure not to pass them to old GCC versions, since that will generate a new warning" I do agree it may be generally useful to have a configurable way to specify pragmas to ignore at runtime, but that is not what I was trying to accomplish here. Both clang and MSVC handle this pragma without any runtime configuration, and I think GCC should as well. Austin On Thu, Nov 12, 2020 at 11:25 PM Jeff Law wrote: > > > On 9/2/20 6:59 PM, Austin Morton via Gcc-patches wrote: > > #pragma region is a feature introduced by Microsoft in order to allow > > manual grouping and folding of code within Visual Studio. It is > > entirely ignored by the compiler. Clang has supported this feature > > since 2012 when in MSVC compatibility mode, and enabled it across the > > board in 2018. > > > > As it stands, you cannot use #pragma region within GCC without > > disabling unknown pragma warnings, which is not advisable. > > > > I propose GCC adopt "#pragma region" and "#pragma endregion" in order > > to alleviate these issues. Because the pragma has no purpose at > > compile time, the implementation is trivial. > > > > > > Microsoft Documentation on the feature: > > https://docs.microsoft.com/en-us/cpp/preprocessor/region-endregion > > > > LLVM change which enabled pragma region across the board: > > https://reviews.llvm.org/D42248 > > --- > > gcc/ChangeLog| 5 + > > gcc/c-family/ChangeLog | 5 + > > gcc/c-family/c-pragma.c | 10 ++ > > gcc/doc/cpp.texi | 6 ++ > > gcc/testsuite/ChangeLog | 5 + > > gcc/testsuite/gcc.dg/pragma-region.c | 21 + > > 6 files changed, 52 insertions(+) > > create mode 100644 gcc/testsuite/gcc.dg/pragma-region.c > > I'm not sure that this is really the way we want to handle this stuff. > I understand the problem you're trying to solve, but embedding a list of > pragmas to ignore into the compiler itself just seems like the wrong > approach -- it bakes that set of pragmas to ignore into the compiler. > > > ISTM that we'd be better off either having a command line option to list > the set of pragmas to ignore, or they should be pulled from a file > specified on the command line. That would seem to be a lot more > friendly to downstream users since each project could set the list of > pragmas to ignore on their own and have that set updated dynamically > over time without having to patch and update GCC. > > > Any chance you would be willing to work on that? > > Jeff >
Re: [PATCH v2] c: Silently ignore pragma region [PR85487]
> But in that case the pragma shouldn't be ignored, but instead checked > against the various requirements. E.g. that region is followed by a single > optional name (are there any requirements on what name can be, can it be > just a single token, can it be C/C++ keyword, etc.), guess ignore all the > tokens after endregion if it is all a comment, and verify nesting > (all region/endregion are properly paired up). I cannot speak to the internals of the MSVC compiler, but I can say definitively that clang does not do any of this. The clang handling of #pragma region is a no-op, as can be seen here: https://github.com/llvm/llvm-project/blob/48b510c4bc0fe090e635ee0440e46fc176527d7e/clang/lib/Lex/Pragma.cpp#L1847 Additionally, the MSVC implementation appears to do no validation (or at least doesn't emit warnings that indicate that it does). https://godbolt.org/z/3zTbnn #pragma region is purely designed for editors to assist in code-folding (primarily visual studio and visual studio code, although I am sure others support it). The goal of this patch is to make GCC compatible with both clang and MSVC handling of this pragma, not to introduce novel functionality.
Re: [PATCH v2] c: Silently ignore pragma region [PR85487]
> How much does this pragma get used "in the wild"? A quick search on github for "#pragma region" comes back with 170k C++ results and searching for "#pragma" comes back with 38M C++ results Possibly not the best metric, but we can "conclude" roughly 0.45% of open source C++ code files on github using any pragmas makes use of #pragma region. https://github.com/search?l=C%2B%2B&q=%22%23pragma+region%22&type=Code https://github.com/search?l=C%2B%2B&q=%22%23pragma%22&type=Code
[PATCH] c: Silently ignore pragma region [PR85487]
#pragma region is a feature introduced by Microsoft in order to allow
manual grouping and folding of code within Visual Studio. It is
entirely ignored by the compiler. Clang has supported this feature
since 2012 when in MSVC compatibility mode, and enabled it across the
board 3 months ago.
As it stands, you cannot use #pragma region within GCC without
disabling unknown pragma warnings, which is not advisable.
I propose GCC adopt "#pragma region" and "#pragma endregion" in order
to alleviate these issues. Because the pragma has no purpose at
compile time, the implementation is trivial.
Microsoft Documentation on the feature:
https://docs.microsoft.com/en-us/cpp/preprocessor/region-endregion
LLVM change which enabled pragma region across the board:
https://reviews.llvm.org/D42248
---
gcc/c-family/ChangeLog | 5 +
gcc/c-family/c-pragma.c | 10 ++
gcc/testsuite/ChangeLog | 5 +
gcc/testsuite/gcc.dg/pragma-region.c | 21 +
4 files changed, 41 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pragma-region.c
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 1eaa99f31..97ba259cd 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-27 Austin Morton
+
+ PR c/85487
+ * c-pragma.c (handle_pragma_region): Declare.
+
2020-08-11 Jakub Jelinek
PR c/96545
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index e3169e68f..de0411d07 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1166,6 +1166,13 @@ handle_pragma_message (cpp_reader *ARG_UNUSED(dummy))
TREE_STRING_POINTER (message));
}
+/* Silently ignore region pragmas. */
+
+static void
+handle_pragma_region (cpp_reader *ARG_UNUSED(dummy))
+{
+}
+
/* Mark whether the current location is valid for a STDC pragma. */
static bool valid_location_for_stdc_pragma;
@@ -1584,6 +1591,9 @@ init_pragma (void)
c_register_pragma_with_expansion (0, "message", handle_pragma_message);
+ c_register_pragma (0, "region", handle_pragma_region);
+ c_register_pragma (0, "endregion", handle_pragma_region);
+
#ifdef REGISTER_TARGET_PRAGMAS
REGISTER_TARGET_PRAGMAS ();
#endif
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5c1a45716..4033c111c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-27 Austin Morton
+
+ PR c/85487
+ * gcc.dg/pragma-region.c: New test.
+
2020-08-26 Jeff Law
* gcc.target/i386/387-7.c: Add dg-require-effective-target c99_runtime.
diff --git a/gcc/testsuite/gcc.dg/pragma-region.c
b/gcc/testsuite/gcc.dg/pragma-region.c
new file mode 100644
index 0..72cc2c144
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pragma-region.c
@@ -0,0 +1,21 @@
+/* Verify #pragma region and #pragma endregion do not emit warnings. */
+
+/* { dg-options "-Wunknown-pragmas" } */
+
+#pragma region
+
+#pragma region name
+
+#pragma region "name"
+
+#pragma region()
+
+#pragma region("name")
+
+#pragma endregion
+
+#pragma endregion garbage
+
+#pragma endregion()
+
+#pragma endregion("garbage")
--
2.17.1
Re: [PATCH] c: Silently ignore pragma region [PR85487]
> The patch misses documentation of the pragma. This was an intentional omission - when looking for documentation of the pragma in clang I found none. If we do want to document the pragmas in GCC: - what section of the documentation would that go in? - gcc/doc/cpp.texi, section 7, "Pragmas" - this section states: "This manual documents the pragmas which are meaningful to the preprocessor itself. Other pragmas are meaningful to the C or C++ compilers. They are documented in the GCC manual." - these pragmas aren't exactly meaningful to the preprocessor _or_ the C/C++ compiler, so it's not exactly clear where they fit. - gcc/doc/extend.texi, section 6.62, "Pragmas Accepted by GCC" - what exactly would it say? - I guess just mention the existence and that they won't trigger unknown pragma warnings? They don't _do_ anything Austin
[PATCH v2] c: Silently ignore pragma region [PR85487]
#pragma region is a feature introduced by Microsoft in order to allow
manual grouping and folding of code within Visual Studio. It is
entirely ignored by the compiler. Clang has supported this feature
since 2012 when in MSVC compatibility mode, and enabled it across the
board in 2018.
As it stands, you cannot use #pragma region within GCC without
disabling unknown pragma warnings, which is not advisable.
I propose GCC adopt "#pragma region" and "#pragma endregion" in order
to alleviate these issues. Because the pragma has no purpose at
compile time, the implementation is trivial.
Microsoft Documentation on the feature:
https://docs.microsoft.com/en-us/cpp/preprocessor/region-endregion
LLVM change which enabled pragma region across the board:
https://reviews.llvm.org/D42248
---
gcc/ChangeLog| 5 +
gcc/c-family/ChangeLog | 5 +
gcc/c-family/c-pragma.c | 10 ++
gcc/doc/cpp.texi | 6 ++
gcc/testsuite/ChangeLog | 5 +
gcc/testsuite/gcc.dg/pragma-region.c | 21 +
6 files changed, 52 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pragma-region.c
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9db853dcd..d0ba77b55 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2020-09-02 Austin Morton
+
+ PR c/85487
+ * doc/cpp.texi (Pragmas): Document pragma region/endregion
+
2020-08-26 Göran Uddeborg
PR gcov-profile/96285
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 1eaa99f31..ccf06095f 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,8 @@
+2020-09-02 Austin Morton
+
+ PR c/85487
+ * c-pragma.c (handle_pragma_region): Declare.
+
2020-08-11 Jakub Jelinek
PR c/96545
diff --git a/gcc/c-family/c-pragma.c b/gcc/c-family/c-pragma.c
index e3169e68f..de0411d07 100644
--- a/gcc/c-family/c-pragma.c
+++ b/gcc/c-family/c-pragma.c
@@ -1166,6 +1166,13 @@ handle_pragma_message (cpp_reader *ARG_UNUSED(dummy))
TREE_STRING_POINTER (message));
}
+/* Silently ignore region pragmas. */
+
+static void
+handle_pragma_region (cpp_reader *ARG_UNUSED(dummy))
+{
+}
+
/* Mark whether the current location is valid for a STDC pragma. */
static bool valid_location_for_stdc_pragma;
@@ -1584,6 +1591,9 @@ init_pragma (void)
c_register_pragma_with_expansion (0, "message", handle_pragma_message);
+ c_register_pragma (0, "region", handle_pragma_region);
+ c_register_pragma (0, "endregion", handle_pragma_region);
+
#ifdef REGISTER_TARGET_PRAGMAS
REGISTER_TARGET_PRAGMAS ();
#endif
diff --git a/gcc/doc/cpp.texi b/gcc/doc/cpp.texi
index 33f876ab7..c868ed695 100644
--- a/gcc/doc/cpp.texi
+++ b/gcc/doc/cpp.texi
@@ -3789,6 +3789,12 @@ file will never be read again, no matter what.
It is a less-portable
alternative to using @samp{#ifndef} to guard the contents of header files
against multiple inclusions.
+@item #pragma region
+@itemx #pragma endregion
+These pragmas are silently ignored by the compiler. Any trailing text is
+also silently ignored. They exist only to facilitate code organization
+and folding in supported editors.
+
@end ftable
@node Other Directives
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5c1a45716..d90067555 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-09-02 Austin Morton
+
+ PR c/85487
+ * gcc.dg/pragma-region.c: New test.
+
2020-08-26 Jeff Law
* gcc.target/i386/387-7.c: Add dg-require-effective-target c99_runtime.
diff --git a/gcc/testsuite/gcc.dg/pragma-region.c
b/gcc/testsuite/gcc.dg/pragma-region.c
new file mode 100644
index 0..72cc2c144
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pragma-region.c
@@ -0,0 +1,21 @@
+/* Verify #pragma region and #pragma endregion do not emit warnings. */
+
+/* { dg-options "-Wunknown-pragmas" } */
+
+#pragma region
+
+#pragma region name
+
+#pragma region "name"
+
+#pragma region()
+
+#pragma region("name")
+
+#pragma endregion
+
+#pragma endregion garbage
+
+#pragma endregion()
+
+#pragma endregion("garbage")
--
2.17.1
