Bug 90400 - _Pragma not always expanded in the right location within macros
Summary: _Pragma not always expanded in the right location within macros
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: preprocessor (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 91517 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-05-08 21:56 UTC by Remi
Modified: 2023-10-02 22:26 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-08-25 00:00:00


Attachments
_Pragma tests (262 bytes, text/plain)
2019-05-20 12:03 UTC, Pekka S
Details
Draft patch for the 'gcc -E' / 'gcc -save-temps' issue (718 bytes, text/plain)
2021-10-29 23:26 UTC, Tobias Burnus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Remi 2019-05-08 21:56:32 UTC
When the preprocessor does not understand the pragma inside _Pragma it calls cb_def_pragma to print it (converting it to '#pragma'). Unfortunately that function uses puts() to print the pragma directly to file.

If we are within a macro being expanded inside another macro then this will results in all pragma within that macro being expanded at the top instead of at their original location within the macro.

For example I would expect the following test to pass:
/* { dg-do assemble }
 * { dg-additional-options "-save-temps -Wall" }
 */


extern int b;

#define OUTPUT(stmt) \
  stmt

#define WITH_PRAGMA() \
  _Pragma("GCC diagnostic push"); \
  _Pragma("GCC diagnostic ignored \"-Wmaybe-uninitialized\""); \
  a--; \
  _Pragma("GCC diagnostic pop")


int main()
{
  int a;

  if (b) {
    b++;
  }
  OUTPUT( else {
    b--;
    WITH_PRAGMA();
  } )
}

Instead with trunk gcc I get:
test.c: In function 'main':
test.c:28:3: error: 'else' without a previous 'if'

Because the pragma statements are printed right at the beginning of output instead of where WITH_PRAGMA is.
Comment 1 Eric Gallager 2019-05-09 04:34:27 UTC
I think there's another bug like this; forget its number right now though...
Comment 2 Remi 2019-05-10 15:19:24 UTC
I found 69543 which looks similar but is different (and fixed): the cause of the bug is different and it applies to the first level of a macro, while this bug requires 2 levels of macro to show up.
Comment 3 Pekka S 2019-05-20 12:03:22 UTC
Created attachment 46384 [details]
_Pragma tests

These all yield different results, the first two won't compile but fail on different lines, which is a bit strange, as the preprocessor output doesn't appear to change (e.g. if only doing -E).

The last one compiles, but obviously just because it omits the pragmas. GCC10 and GCC7 behave both the same with this test setup, so I suspect this is a long standing feature and/or issue.

gcc -Wall -Wextra -c pragma.c
gcc -Wall -Wextra -save-temps -c pragma.c
gcc -Wall -Wextra -save-temps -DNOPRAGMA -c pragma.c
Comment 4 Tobias Burnus 2021-10-29 17:51:36 UTC
If I look at the 'gcc -E' output, the order is reverted:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wall"
#pragma GCC diagnostic pop
  else { b--; ; ; a--; ; }

this should be instead:

  else { b--;
#pragma GCC diagnostic push
;
#pragma GCC diagnostic ignored "-Wall"
; a--;
#pragma GCC diagnostic pop
; }


I did observe the same in PR102409 – see analysis there. But contrary to the issue in the other PR, the patch there does not solve the issue in this PR.

Additionally, for 'GCC diagnostic' there might be a column issue as discussed in PR91669 comment 3.
Comment 5 Tobias Burnus 2021-10-29 22:09:01 UTC
The problem is that the pragma is not known/registered. In that case, when calling
libcpp/directives.c's do_pragma, the result is  p == NULL

and thus:

  if (p)
    ...
  else if (pfile->cb.def_pragma)
    ...
    pfile->cb.def_pragma (pfile, pfile->directive_line);

the latter immediately prints the '#pragma ...'

I think what needs to be done is if (p == NULL && pfile->state.in_directive) to create a new pragma on the fly and store it in pfile->directive_result to use it later.

 * * *

Side note: For 'gcc -E c-c++-common/gomp/pragma-1.c' the same issue occurs, but if one adds  -fopenmp, p != NULL and everything is fine.
Comment 6 Tobias Burnus 2021-10-29 23:26:06 UTC
Created attachment 51700 [details]
Draft patch for the 'gcc -E' / 'gcc -save-temps' issue

This solves the -E / -save-temps preprocessing issue.

For the non -E issue, it could be the issue described at PR 91669.
Comment 7 Tobias Burnus 2021-11-10 11:54:19 UTC
(In reply to Tobias Burnus from comment #6)
> Created attachment 51700 [details]
> Draft patch for the 'gcc -E' / 'gcc -save-temps' issue
> 
> This solves the -E / -save-temps preprocessing issue.
> 
> For the non -E issue, it could be the issue described at PR 91669.

Regarding "-E": Actually, looking at PR103165 comment 2, I note that for the example there 'clang -E' outputs:

"hello; \"\" _Pragma(\"GCC diagnostic pop\") world;"

that is: While the macros are replaced, the (unknown) _Pragma remains as _Pragma - such that it is then later only processed when running the compiler.

No idea whether that makes sense or not not - just as observation.
Comment 8 Lewis Hyatt 2023-08-25 20:17:12 UTC
(In reply to Tobias Burnus from comment #7)
> Regarding "-E": Actually, looking at PR103165 comment 2, I note that for the
> example there 'clang -E' outputs:
> 
> "hello; \"\" _Pragma(\"GCC diagnostic pop\") world;"
> 
> that is: While the macros are replaced, the (unknown) _Pragma remains as
> _Pragma - such that it is then later only processed when running the
> compiler.
> 
> No idea whether that makes sense or not not - just as observation.

This issue was indeed fixed by r12-5454, the fix for PR103165.

I will get a testcase added and then close this one. The testcase will be a tweaked version of the original one from this PR. It needs to use a different _Pragma, because nowadays, '#pragma GCC diagnostic' is recognized by the preprocessor. The existing c-c++-common/gomp/pragma-2.c provides coverage for that case. '#pragma GCC unroll' is a useful new testcase, being another pragma that is explicitly ignored during preprocess-only modes.
Comment 9 GCC Commits 2023-09-20 20:46:41 UTC
The master branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>:

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

commit r14-4186-gd8e08ba9396b1f7da50011468f260250b7afaab7
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Fri Aug 25 15:57:19 2023 -0400

    testsuite: Add test for already-fixed issue with _Pragma expansion [PR90400]
    
    The PR was fixed by r12-5454. Since the fix was somewhat incidental,
    although related, add a testcase from PR90400 too before closing it out.
    
    gcc/testsuite/ChangeLog:
    
            PR preprocessor/90400
            * c-c++-common/cpp/pr90400.c: New test.
Comment 10 Lewis Hyatt 2023-09-20 20:51:44 UTC
Marking it fixed now that the testcase is added.
Comment 11 Lewis Hyatt 2023-10-02 16:57:34 UTC
*** Bug 91517 has been marked as a duplicate of this bug. ***