Thread

Commits

  1. Refactor static_assert() support.

  2. Replace most StaticAssertStmt() with StaticAssertDecl()

  3. Make <assert.h> consistently available in frontend and backend

  1. Confused static assertion implementation

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-14T02:13:02Z

    Hi,
    
    I won't be surprised if Peter (CC'd) already has a patch for this in
    his C11 incubation branch, but if not, maybe this will be useful.
    
    While experimenting with threads and atomics on various systems, I
    didn't like the way our macros failed in the ancient fallback code on
    Visual Studio.  That only seems to be necessary for
    StaticAssertExpr(), the rarest case.  That led to some observations:
    
    * the meson script defines HAVE__STATIC_ASSERT if you have GCC
    statement expressions, so why not just say so with
    HAVE_STATEMENT_EXPRESSIONS, and keep ye olde fallback only for
    StaticAssertExpr()?
    
    * the configure script is different, tautological and wrong for
    (evidently hypothetical) non-GCC-compatible C11 compiler given the
    above interpretation of the macro
    
    * to my knowledge we haven't written down which C++ standard our
    headers conform to anywhere, but it surely can't be older than C++11
    (I could elaborate), so I don't think we need the two __cplusplus
    implementations
    
    Here's an attempt to tidy that up.
    
    We could also consider allowing ourselves to use standard
    static_assert() directly in new code, in scopes that accept
    declarations (eg file top-level, inside structs, inside functions as
    allowed by our self-imposed -Wno-declaration-after-statement rule),
    but that's essentially an independent question and I can also see that
    the inconsistency might be annoying, cf size_t/Size, uint64_t/uint64,
    etc debates with different outcomes.  For reference, we currently have
    84 "...Decl", 10 "...Expr", and 18 "...Stmt" occurrences in the tree,
    and if that has any predictive power, the vast majority of new code
    would likely just use static_assert().
    
    . o O { I wonder if it's possible to write a C23/C++17-compatible
    single-argument static_assert() macro, or if you'd get stuck in a loop
    and need to use a different name... the message argument is so often
    boring/redundant... }
    
  2. Re: Confused static assertion implementation

    Chao Li <li.evan.chao@gmail.com> — 2025-11-14T05:17:47Z

    
    > On Nov 14, 2025, at 10:13, Thomas Munro <thomas.munro@gmail.com> wrote:
    > 
    > Hi,
    > 
    > I won't be surprised if Peter (CC'd) already has a patch for this in
    > his C11 incubation branch, but if not, maybe this will be useful.
    > 
    > While experimenting with threads and atomics on various systems, I
    > didn't like the way our macros failed in the ancient fallback code on
    > Visual Studio.  That only seems to be necessary for
    > StaticAssertExpr(), the rarest case.  That led to some observations:
    > 
    > * the meson script defines HAVE__STATIC_ASSERT if you have GCC
    > statement expressions, so why not just say so with
    > HAVE_STATEMENT_EXPRESSIONS, and keep ye olde fallback only for
    > StaticAssertExpr()?
    > 
    
    +1, I think this is clearer.
    
    > 
    > . o O { I wonder if it's possible to write a C23/C++17-compatible
    > single-argument static_assert() macro, or if you'd get stuck in a loop
    > and need to use a different name... the message argument is so often
    > boring/redundant... }
    > <0001-Refactor-static_assert-support.patch>
    
    A few small comments:
    
    1
    ```
     main ()
     {
    -({ _Static_assert(1, "foo"); })
    +({ _Static_assert(1, "foo"); });
    +
       ;
       return 0;
     }
    ```
    
    As you added a semi-colon in the line, the one after the empty line can be deleted, though C allows empty statement, but unnecessary, and may lead to confusion for code readers.
    
    2
    ```
    -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__static_assert" >&5
    -$as_echo "$pgac_cv__static_assert" >&6; }
    +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__statement_expressions" >&5
    +$as_echo "$pgac_cv__statement_expressions" >&6; }
     if test x"$pgac_cv__static_assert" = xyes ; then
    ```
    
    You missed to replace this pgac_cv__static_assert with the new name.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  3. Re: Confused static assertion implementation

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-14T22:27:23Z

    On Fri, Nov 14, 2025 at 6:18 PM Chao Li <li.evan.chao@gmail.com> wrote:
    > As you added a semi-colon in the line, the one after the empty line can be deleted, though C allows empty statement, but unnecessary, and may lead to confusion for code readers.
    
    > You missed to replace this pgac_cv__static_assert with the new name.
    
    Ugh, yeah, the configure change was hopeless.  It looked like it
    worked in configure's stdout, which I mistook for success and posted
    too soon, sorry about that.  I have fixed those points and verified
    that pg_config.h actually has the expected value.
    
    Thanks for the review!
    
  4. Re: Confused static assertion implementation

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-18T13:31:10Z

    On 14.11.25 03:13, Thomas Munro wrote:
    > I won't be surprised if Peter (CC'd) already has a patch for this in
    > his C11 incubation branch, but if not, maybe this will be useful.
    > 
    > While experimenting with threads and atomics on various systems, I
    > didn't like the way our macros failed in the ancient fallback code on
    > Visual Studio.  That only seems to be necessary for
    > StaticAssertExpr(), the rarest case.  That led to some observations:
    > 
    > * the meson script defines HAVE__STATIC_ASSERT if you have GCC
    > statement expressions, so why not just say so with
    > HAVE_STATEMENT_EXPRESSIONS, and keep ye olde fallback only for
    > StaticAssertExpr()?
    > 
    > * the configure script is different, tautological and wrong for
    > (evidently hypothetical) non-GCC-compatible C11 compiler given the
    > above interpretation of the macro
    
    There is a curious, now obsolete surprise here: MSVC without C11 mode 
    enabled does support _Static_assert, but it does not allow it at file 
    scope.  Our overly restrictive HAVE__STATIC_ASSERT check fails on MSVC, 
    but if it passed our code would have failed to compile because of the 
    file-scope static assertions.  So this one suboptimal check covered for 
    a lack of a check elsewhere.
    
    This is now obsolete; with MSVC in C11 mode, _Static_assert appears to 
    behave correctly.
    
    I have found that the C standard committee is actively researching 
    allowing static assertions in expressions:
    
    https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3538.pdf
    https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3637.pdf
    https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3682.pdf
    https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3715.pdf
    
    Those documents also contain some information about workarounds in use.
    
    Btw., statement expressions are also in play:
    
    https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3643.htm
    
    But it looks like the most popular workaround is to put the 
    static_assert in a struct declaration.
    
    On compiler explorer, I found that this works in MSVC but produces a 
    warning.  But in my actual PostgreSQL patches it currently fails.  But 
    anyway, since we're talking about it now, I'm attaching my patches here. 
      So only 0001 through 0004 work, the remaining two are WIP.  Maybe 
    someone can stare at 0005 long enough to find what the problem is.
    
    > * to my knowledge we haven't written down which C++ standard our
    > headers conform to anywhere, but it surely can't be older than C++11
    > (I could elaborate), so I don't think we need the two __cplusplus
    > implementations
    
    Per 
    <https://www.postgresql.org/message-id/01a69441-af54-4822-891b-ca28e05b215a%40eisentraut.org> 
    it's >= C++11.
    
    > . o O { I wonder if it's possible to write a C23/C++17-compatible
    > single-argument static_assert() macro, or if you'd get stuck in a loop
    > and need to use a different name... the message argument is so often
    > boring/redundant... }
    
    That would be nice, but I haven't tried it yet.
    
  5. Re: Confused static assertion implementation

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-15T11:58:40Z

    On 14.11.25 23:27, Thomas Munro wrote:
    > On Fri, Nov 14, 2025 at 6:18 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >> As you added a semi-colon in the line, the one after the empty line can be deleted, though C allows empty statement, but unnecessary, and may lead to confusion for code readers.
    > 
    >> You missed to replace this pgac_cv__static_assert with the new name.
    > 
    > Ugh, yeah, the configure change was hopeless.  It looked like it
    > worked in configure's stdout, which I mistook for success and posted
    > too soon, sorry about that.  I have fixed those points and verified
    > that pg_config.h actually has the expected value.
    
    I have committed this, with some light editing of the comments.
    
    We are now down to 4 from previously 12 static assertion implementation 
    variants!
    
    Note, however, that the now committed non-statement-expression fallback 
    implementation of StaticAssertExpr() does not work under C++.  (The 
    compiler complains about "types may not be defined in 'sizeof' 
    expressions".)  This isn't a regression in the overall sense, since, as 
    you had mentioned, previously all C++ variants required statement 
    expressions.  So I think this can be added to the list of C++ 
    portability issues that Jelte is currently working through.