Re: [patch] Use __builtin_FILE and __builtin_LINE in assert implementation in C++

[Florian Weimer <fweimer@redhat.com>]

* Rich Felker:

> On Mon, Jan 23, 2023 at 03:26:04PM +0100, Florian Weimer via Libc-alpha wrote:
>> * Paul Pluzhnikov via Libc-alpha:
>> 
>> > diff --git a/assert/assert.h b/assert/assert.h
>> > index 72209bc5e7..4e0303db8d 100644
>> > --- a/assert/assert.h
>> > +++ b/assert/assert.h
>> > @@ -89,7 +89,8 @@ __END_DECLS
>> >  #  define assert(expr)                                                 \
>> >       (static_cast <bool> (expr)
>> >          \
>> >        ? void (0)                                                       \
>> > -      : __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION))
>> > +      : __assert_fail (#expr, __builtin_FILE (), __builtin_LINE (),     \
>> > +                       __ASSERT_FUNCTION))
>> >  # elif !defined __GNUC__ || defined __STRICT_ANSI__
>> >  #  define assert(expr)                                                 \
>> >      ((expr)                                                            \
>> 
>> I think __builtin_FILE and __builtin_LINE are farily recent GCC/Clang
>> additions, so they need compiler version checks or a __has_builtin gate.
>> 
>> Ideally, we would use <source_location> here, but I don't think we can
>> include that from <assert.h>.
>> 
>> Thanks,
>> Florian
>
> Why not instead:
>
> +static char *__file_for_assert = __FILE__;
> ...
> -      : __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION))
> +      : __assert_fail (#expr, __file_for_assert, __LINE__, __ASSERT_FUNCTION))
>
> ??

__FILE__ expansion needs to be delayed, otherwise it refers to assert.h.

I think the builtins also have the advantage that they avoid ODR
violations because the definition is the same no matter what the file is
called.  This issue is not restricted to modules.  But maybe the right
answer is to blame the programmer and just add a diagnostic?

Jonathan, I think the libstdc++ headers have the same issue.  Thoughts?

Thanks,
Florian