[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[bmburstein at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

Baruch Burstein <bmburstein at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bmburstein at gmail dot com

--- Comment #2 from Baruch Burstein <bmburstein at gmail dot com> ---
This pragma is used by VSCode, too, which is commonly used with gcc as a
compiler.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |easyhack

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The docs raise some questions.

They say that a #pragma region must be ended by a #pragma endregion. Should the
compiler check that and issue a diagnostic otherwise?

What is the form of the optional "name" that follows #pragma region?

What if #pragma endregion is followed by preprocessor tokens, not just a
comment?

If we don't care about validating anything, it's easy to make GCC completely
ignore those pragmas:

--- a/gcc/c-family/c-pragma.cc
+++ b/gcc/c-family/c-pragma.cc
@@ -1218,6 +1218,15 @@ handle_pragma_message (cpp_reader *ARG_UNUSED(dummy))
            TREE_STRING_POINTER (message));
 }

+/* Ignore a no-op pragma that GCC recognizes, but which has no effect.  */
+static void
+handle_pragma_ignore (cpp_reader *)
+{
+  tree x;
+  while (pragma_lex (&x) != CPP_EOF)
+    /* Ignore the rest of the line.  */;
+}
+
 /* Mark whether the current location is valid for a STDC pragma.  */

 static bool valid_location_for_stdc_pragma;
@@ -1633,6 +1642,9 @@ init_pragma (void)
   c_register_pragma ("GCC", "pop_options", handle_pragma_pop_options);
   c_register_pragma ("GCC", "reset_options", handle_pragma_reset_options);

+  c_register_pragma (0, "region", handle_pragma_ignore);
+  c_register_pragma (0, "endregion", handle_pragma_ignore);
+
   c_register_pragma ("STDC", "FLOAT_CONST_DECIMAL64",
                     handle_pragma_float_const_decimal64);



This needs tests though.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Exactly.  Seems clang doesn't bother with it and allows anything, but it is
unclear if that is the best thing to do.
Is name and comment just a single identifier, or any sequence of any tokens
until end of line?
And, shall the compiler verify proper nestin of these, e.g. reject
#pragma endregion
without earlier #pragma region, or
#pragma region
without termination, or whenever they aren't properly nested?

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[austinpmorton at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #5 from Austin Morton <austinpmorton at gmail dot com> ---

(In reply to Jonathan Wakely from comment #3)
> The docs raise some questions.
> 
> They say that a #pragma region must be ended by a #pragma endregion. Should
> the compiler check that and issue a diagnostic otherwise?
> 
> What is the form of the optional "name" that follows #pragma region?
> 
> What if #pragma endregion is followed by preprocessor tokens, not just a
> comment?
> 
> If we don't care about validating anything, it's easy to make GCC completely
> ignore those pragmas:
> 
> --- a/gcc/c-family/c-pragma.cc
> +++ b/gcc/c-family/c-pragma.cc
> @@ -1218,6 +1218,15 @@ handle_pragma_message (cpp_reader *ARG_UNUSED(dummy))
>             TREE_STRING_POINTER (message));
>  }
>  
> +/* Ignore a no-op pragma that GCC recognizes, but which has no effect.  */
> +static void
> +handle_pragma_ignore (cpp_reader *)
> +{
> +  tree x;
> +  while (pragma_lex (&x) != CPP_EOF)
> +    /* Ignore the rest of the line.  */;
> +}
> +
>  /* Mark whether the current location is valid for a STDC pragma.  */
>  
>  static bool valid_location_for_stdc_pragma;
> @@ -1633,6 +1642,9 @@ init_pragma (void)
>    c_register_pragma ("GCC", "pop_options", handle_pragma_pop_options);
>    c_register_pragma ("GCC", "reset_options", handle_pragma_reset_options);
>  
> +  c_register_pragma (0, "region", handle_pragma_ignore);
> +  c_register_pragma (0, "endregion", handle_pragma_ignore);
> +
>    c_register_pragma ("STDC", "FLOAT_CONST_DECIMAL64",
>                      handle_pragma_float_const_decimal64);
>  
> 
> 
> This needs tests though.

https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553182.html

I sent a patch to do exactly that in 2020 and it was not accepted.

This seems like a very easy win.
Both major competitors to GCC (clang and MSVC) implement this pragma exactly
like in my patch (by completely ignoring it).

As it stands today, this is plainly a deficiency in GCC when compared to its
competition.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> Exactly.  Seems clang doesn't bother with it and allows anything, but it is
> unclear if that is the best thing to do.

MSVC ignores any tokens after the region/endregion as well.

> Is name and comment just a single identifier, or any sequence of any tokens
> until end of line?
> And, shall the compiler verify proper nestin of these, e.g. reject
> #pragma endregion
> without earlier #pragma region, or
> #pragma region
> without termination, or whenever they aren't properly nested?

Again, other compilers don't care:
https://godbolt.org/z/zxhYaaz9Y

So it looks like this is completely ignored by the compiler, and any logic
relating to it exists only in the editor(s).

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I would say that is a terrible design...

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Austin Morton from comment #5)
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553182.html
> 
> I sent a patch to do exactly that in 2020 and it was not accepted.

Thanks. The review thread continues in November (but the mail archive doesn't
thread it properly across months):

https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558900.html

I see what Jeff is saying, but I think the pragmatic approach here is to just
do exactly what your patch does: hardcode this warning as a no-op. We don't
have to do that for every pragma implemented by every compiler, because giving
a -Wunknown-pragmas warning about *unknown* pragmas is useful. GCC doesn't know
what the pragma was attempting to do, so it should warn.

But we do know what this one does: nothing. It's not unknown.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> I would say that is a terrible design...

Yes, I completely agree, but I don't see why GCC should be in the business of
diagnosing other people's junk :-)

Maybe Visual Studio's editor and VScode do have checks, just not the VC++
compiler. And if so, then that's even more reason that we don't need GCC to do
its own checking.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=61593
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #10 from Eric Gallager <egallager at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #3)
> The docs raise some questions.
> 
> They say that a #pragma region must be ended by a #pragma endregion. Should
> the compiler check that and issue a diagnostic otherwise?
> 
> What is the form of the optional "name" that follows #pragma region?
> 
> What if #pragma endregion is followed by preprocessor tokens, not just a
> comment?
> 
> If we don't care about validating anything, it's easy to make GCC completely
> ignore those pragmas:
> 
> --- a/gcc/c-family/c-pragma.cc
> +++ b/gcc/c-family/c-pragma.cc
> @@ -1218,6 +1218,15 @@ handle_pragma_message (cpp_reader *ARG_UNUSED(dummy))
>             TREE_STRING_POINTER (message));
>  }
>  
> +/* Ignore a no-op pragma that GCC recognizes, but which has no effect.  */
> +static void
> +handle_pragma_ignore (cpp_reader *)
> +{
> +  tree x;
> +  while (pragma_lex (&x) != CPP_EOF)
> +    /* Ignore the rest of the line.  */;
> +}
> +
>  /* Mark whether the current location is valid for a STDC pragma.  */
>  
>  static bool valid_location_for_stdc_pragma;
> @@ -1633,6 +1642,9 @@ init_pragma (void)
>    c_register_pragma ("GCC", "pop_options", handle_pragma_pop_options);
>    c_register_pragma ("GCC", "reset_options", handle_pragma_reset_options);
>  
> +  c_register_pragma (0, "region", handle_pragma_ignore);
> +  c_register_pragma (0, "endregion", handle_pragma_ignore);
> +
>    c_register_pragma ("STDC", "FLOAT_CONST_DECIMAL64",
>                      handle_pragma_float_const_decimal64);
>  
> 
> 
> This needs tests though.

so, this seems relevant to bug 61593 too...

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

Florian Weimer <fw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fw at gcc dot gnu.org

--- Comment #11 from Florian Weimer <fw at gcc dot gnu.org> ---
Clang does not appear to treat this pragma as a statement. Is this also the
MSVC behavior?

int
f (int i)
{
  if (i)
#pragma region
    return 1;
#pragma endregion
  return 0;
}

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org

--- Comment #12 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> (In reply to Jakub Jelinek from comment #7)
> > I would say that is a terrible design...
> 
> Yes, I completely agree, but I don't see why GCC should be in the business
> of diagnosing other people's junk :-)
> 
> Maybe Visual Studio's editor and VScode do have checks, just not the VC++
> compiler. And if so, then that's even more reason that we don't need GCC to
> do its own checking.
Agreed.  And if people with strict linting requirements want
a warning about pragmas that are recognised but have no effect
on the compiler, we could still provide an option to do that
(but it shouldn't be in -Wall or even -Wextra).

That shouldn't be a requirement for this PR though, unless anyone
can show that someone somewhere really does want these pragmas to
generate a warning.

Jeff said at the end of the thread that he wouldn't mind
if someone else approves it, so it's probably worth posting
again.  The patch LGTM FWIW: only (very) minor comment is that
the unused argument name in handle_pragma_region can be dropped.

I think the patch would need to wait for GCC 13 now though.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #12)
> Jeff said at the end of the thread that he wouldn't mind
> if someone else approves it, so it's probably worth posting
> again.

OK, will do.

I think Jeff's desire for a framework to ignore arbitrary pragmas might be
nice, but nobody's going to do that, and this small improvement has a working
patch. We shouldn't let the perfect be the enemy of the good.

> The patch LGTM FWIW: only (very) minor comment is that
> the unused argument name in handle_pragma_region can be dropped.

Yeah, that was consistent with the rest of the file, but I already changed the
rest in r12-7282-g73a118c209fcbb so I'd update my patch in the same way.

I'll also update it to ignore the very similar Xcode pragmas described in PR
61593.

> I think the patch would need to wait for GCC 13 now though.

Indeed.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Florian Weimer from comment #11)
> Clang does not appear to treat this pragma as a statement. Is this also the
> MSVC behavior?

Yes, I think so. I'll check that my patch is consistent.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[bmburstein at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #15 from Baruch Burstein <bmburstein at gmail dot com> ---
(In reply to Jonathan Wakely from comment #13)
> (In reply to rsandifo@gcc.gnu.org from comment #12)
>  
> > I think the patch would need to wait for GCC 13 now though.
> 
> Indeed.

Now that GCC 13 is the main development trunk, can this patch be merged? If I
understood the comments in this thread correctly, the patch already exists and
was just waiting for GCC 12 to be branched.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:d3fe767c16e7c528e4fc71c8a68ac14b4573d880

commit r13-3887-gd3fe767c16e7c528e4fc71c8a68ac14b4573d880
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 9 21:49:52 2022 +0000

    c-family: Support #pragma region/endregion [PR85487]

    These pragmas are used by some editors to mark regions of code for
    grouping and folding. GCC should silently ignore them, rather than
    giving -Wunknown-pragmas warnings.

            PR c/85487

    gcc/ChangeLog:

            * doc/cpp/pragmas.rst (Pragmas): Document region pragmas.

    gcc/c-family/ChangeLog:

            * c-pragma.cc (handle_pragma_ignore): New function.
            (init_pragma): Register region and endregion pragmas.

    gcc/testsuite/ChangeLog:

            * c-c++-common/pragma-region.c: New test.

[Bug c/85487] Support '#pragma region' and '#pragma endregion' to allow code folding with Visual Studio

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85487

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #17 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Implemented for GCC 13.