Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix timespec/timeval warning when compiling C extension modules #525

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

tekknolagi
Copy link
Owner

On some compilers, the warnings below get generated. I think it's
something to do with not defining _POSIX_C_SOURCE when we include
time.h but that didn't work--so it might be a project-level setting
that we need to tweak.

In the meantime, forward declare the structs in cpython-types.h.

In file included from /home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-data.h:5,
                 from /home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/Python.h:15,
                 from signature.c:1:
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1311:58: warning: ‘struct timespec’ declared inside parameter list will not be visible outside of this definition or declaration
 1311 | PyAPI_FUNC_DECL(int _PyTime_AsTimespec(_PyTime_t, struct timespec*));
      |                                                          ^~~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1312:57: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration
 1312 | PyAPI_FUNC_DECL(int _PyTime_AsTimeval(_PyTime_t, struct timeval*,
      |                                                         ^~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1316:65: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration
 1316 | PyAPI_FUNC_DECL(int _PyTime_AsTimeval_noraise(_PyTime_t, struct timeval*,
      |                                                                 ^~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1325:64: warning: ‘struct timespec’ declared inside parameter list will not be visible outside of this definition or declaration
 1325 | PyAPI_FUNC_DECL(int _PyTime_FromTimespec(_PyTime_t* tp, struct timespec* ts));
      |                                                                ^~~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1326:63: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration
 1326 | PyAPI_FUNC_DECL(int _PyTime_FromTimeval(_PyTime_t* tp, struct timeval* tv));
      |                                                               ^~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~

On some compilers, the warnings below get generated. I think it's
something to do with not defining `_POSIX_C_SOURCE` when we include
`time.h` but that didn't work--so it might be a project-level setting
that we need to tweak.

In the meantime, forward declare the structs in `cpython-types.h`.

```
In file included from /home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-data.h:5,
                 from /home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/Python.h:15,
                 from signature.c:1:
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1311:58: warning: ‘struct timespec’ declared inside parameter list will not be visible outside of this definition or declaration
 1311 | PyAPI_FUNC_DECL(int _PyTime_AsTimespec(_PyTime_t, struct timespec*));
      |                                                          ^~~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1312:57: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration
 1312 | PyAPI_FUNC_DECL(int _PyTime_AsTimeval(_PyTime_t, struct timeval*,
      |                                                         ^~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1316:65: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration
 1316 | PyAPI_FUNC_DECL(int _PyTime_AsTimeval_noraise(_PyTime_t, struct timeval*,
      |                                                                 ^~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1325:64: warning: ‘struct timespec’ declared inside parameter list will not be visible outside of this definition or declaration
 1325 | PyAPI_FUNC_DECL(int _PyTime_FromTimespec(_PyTime_t* tp, struct timespec* ts));
      |                                                                ^~~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:1326:63: warning: ‘struct timeval’ declared inside parameter list will not be visible outside of this definition or declaration
 1326 | PyAPI_FUNC_DECL(int _PyTime_FromTimeval(_PyTime_t* tp, struct timeval* tv));
      |                                                               ^~~~~~~
/home/max/Documents/code/skybison/build-relwithdebinfo/include/skybison3.8/cpython-func.h:21:70: note: in definition of macro ‘PyAPI_FUNC_DECL’
   21 | #define PyAPI_FUNC_DECL(DECL) __attribute__((visibility("default"))) DECL
      |                                                                      ^~~~
```
Copy link

{
  "django_minimal_requests": {
    "benchmark": "django_minimal_requests",
    "cg_instructions before": 682502,
    "cg_instructions now": 683318,
    "cg_instructions ∆": "0.1%",
    "interpreter_args": [],
    "interpreter_name": "pyro",
    "version before": "ba3fe617c745c2f297df1b98e90d1020ccbb2eb5",
    "version now": "32960c735767d33f078d4538cc5a4235e52dd6c3"
  }
}

Copy link

Summary

Metric Average Best Worst Notes
cg_instructions 0.0% bench_compile -0.0% loadproperty 0.0% typically < 0.2% noise
Benchmark details

Base vs. New

benchmark cg_instructions
2to3 -0.0%
bench_base64 0.0%
bench_compile -0.0%
bench_pickle 0.0%
deltablue 0.0%
fannkuch 0.0%
go 0.0%
loadproperty 0.0%
nbody 0.0%
nqueens 0.0%
pyflate 0.0%
pystone 0.0%
richards 0.0%

CPython vs New

benchmark cg_instructions
2to3 -10.8%
bench_base64 -40.2%
bench_compile 1076.0%
bench_pickle -28.0%
deltablue -65.3%
fannkuch -3.4%
go -64.6%
loadproperty -75.5%
nbody 20.7%
nqueens 25.5%
pyflate -33.8%
pystone -73.9%
richards -80.1%

Base

benchmark cg_instructions
2to3 2,287,551,572
bench_base64 2,882,313,099
bench_compile 2,905,491,131
bench_pickle 2,985,833,105
deltablue 1,430,221,175
fannkuch 5,397,359,841
go 1,864,328,484
loadproperty 427,313,438
nbody 9,408,474,760
nqueens 2,776,591,010
pyflate 9,583,576,867
pystone 1,038,574,892
richards 957,256,952

New

benchmark cg_instructions
2to3 2,287,515,749
bench_base64 2,882,333,670
bench_compile 2,905,417,821
bench_pickle 2,985,881,979
deltablue 1,430,234,330
fannkuch 5,397,370,561
go 1,864,480,531
loadproperty 427,364,704
nbody 9,408,480,829
nqueens 2,776,598,029
pyflate 9,583,581,271
pystone 1,038,577,078
richards 957,258,955

CPython

benchmark cg_instructions
2to3 2,564,111,609
bench_base64 4,819,171,422
bench_compile 247,069,506
bench_pickle 4,145,157,377
deltablue 4,118,645,788
fannkuch 5,587,461,051
go 5,263,097,076
loadproperty 1,747,800,158
nbody 7,793,866,772
nqueens 2,213,294,569
pyflate 14,472,097,572
pystone 3,981,820,336
richards 4,819,791,013

@tekknolagi tekknolagi marked this pull request as ready for review February 22, 2024 20:59
@tekknolagi tekknolagi merged commit df9b2db into trunk Feb 22, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant