| Message ID | CALoOobP+sGwmAA-14zxN+T_oxcdy-Nhfc-0qECZYsFvFNCKk1A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use __builtin_FILE and __builtin_LINE in assert implementation in C++ | expand |
* 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
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)) ?? No need for any version dependent and nonportable builtins. Rich
On Tue, Jan 24, 2023 at 06:10:20AM -0500, Rich Felker wrote: > 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)) > > ?? > > No need for any version dependent and nonportable builtins. N/m, this of course "works" but prints the wrong file (assert.h). I don't suppose there's any way to fix that; at least I don't see one. Rich
* 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
On Tue, Jan 24, 2023 at 12:19:32PM +0100, Florian Weimer wrote: > * 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? Arguably use of assert() in inline functions subject to this rule is a programming error, since there's no guarantee it expands to the same sequence of tokens as required. My leaning is that no action is required here. (Also, C++ programmers should stop putting so much code in inline functions in headers. That's why C++ programs are so gigantic and take ridiculous amounts of time to build. If it's big enough to merit assertions it's almost surely too big to belong in an inline.) Rich
On Tue, 24 Jan 2023 at 11:38, Rich Felker <dalias@libc.org> wrote: > > On Tue, Jan 24, 2023 at 12:19:32PM +0100, Florian Weimer wrote: > > * 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 Added to GCC 4.8.0 in 2012. I don't know about 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. It's not that simple though, the definition consists of the same sequence of tokens, but can still result in incompatible definitions: char* f() { static char array[__builtin_LINE()]; return array; } c.f. https://cplusplus.github.io/CWG/issues/2678.html > > 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? Yes, we should probably make a similar change. > Arguably use of assert() in inline functions subject to this rule is a > programming error, since there's no guarantee it expands to the same > sequence of tokens as required. My leaning is that no action is > required here. > > (Also, C++ programmers should stop putting so much code in inline > functions in headers. Good luck changing that. > That's why C++ programs are so gigantic and take > ridiculous amounts of time to build. If it's big enough to merit > assertions it's almost surely too big to belong in an inline.) There's plenty of code like this: struct vector { int* data; size_t len; int& operator[](int n) { assert(n < len); return data[len]; } };
* Jonathan Wakely: >>>>> I think __builtin_FILE and __builtin_LINE are farily recent GCC/Clang > > Added to GCC 4.8.0 in 2012. I don't know about Clang. Oh, I didn't realize it's been this long. >> > __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. > > It's not that simple though, the definition consists of the same > sequence of tokens, but can still result in incompatible definitions: > > char* f() > { > static char array[__builtin_LINE()]; > return array; > } > > c.f. https://cplusplus.github.io/CWG/issues/2678.html Ugh, I was worried about something like that. And the location functions need to be constexpr. There's also the issue that <cassert> is defined in terms of ISO C, and ISO C specifies that __FILE__ and __LINE__ must be used. Is that another defect? But maybe the difference is not observable because __FILE__ cannot be redefined? Thanks, Florian
On Tue, 24 Jan 2023 at 12:08, Florian Weimer <fweimer@redhat.com> wrote: > > * Jonathan Wakely: > > >>>>> I think __builtin_FILE and __builtin_LINE are farily recent GCC/Clang > > > > Added to GCC 4.8.0 in 2012. I don't know about Clang. > > Oh, I didn't realize it's been this long. > > >> > __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. > > > > It's not that simple though, the definition consists of the same > > sequence of tokens, but can still result in incompatible definitions: > > > > char* f() > > { > > static char array[__builtin_LINE()]; > > return array; > > } > > > > c.f. https://cplusplus.github.io/CWG/issues/2678.html > > Ugh, I was worried about something like that. And the location > functions need to be constexpr. Yes. But that's not a problem for assert because it can't be used to define objects or constants that would vary between TUs. > There's also the issue that <cassert> is defined in terms of ISO C, > and ISO C specifies that __FILE__ and __LINE__ must be used. Is that > another defect? But maybe the difference is not observable because > __FILE__ cannot be redefined? I don't think it's observable. C requires the text printed by a failed assert to include "the values of the preprocessing macros __FILE__ and __LINE__ " but it doesn't require the actual tokens to appear in the definition. The built-ins do return "the values" of those macros, so that seems OK to me.
The 01/24/2023 12:17, Jonathan Wakely via Libc-alpha wrote: > On Tue, 24 Jan 2023 at 12:08, Florian Weimer <fweimer@redhat.com> wrote: > > There's also the issue that <cassert> is defined in terms of ISO C, > > and ISO C specifies that __FILE__ and __LINE__ must be used. Is that > > another defect? But maybe the difference is not observable because > > __FILE__ cannot be redefined? > > I don't think it's observable. C requires the text printed by a failed > assert to include "the values of the preprocessing macros __FILE__ and > __LINE__ " but it doesn't require the actual tokens to appear in the > definition. The built-ins do return "the values" of those macros, so > that seems OK to me. is that a real fix? the token sequence is the same in different TUs with the builtins but the actual printed path is different, so if the linker picks a definition at random then the produced binary is not deterministic. why is this better than just using __FILE__?
On Tue, 24 Jan 2023 at 15:08, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > The 01/24/2023 12:17, Jonathan Wakely via Libc-alpha wrote: > > On Tue, 24 Jan 2023 at 12:08, Florian Weimer <fweimer@redhat.com> wrote: > > > There's also the issue that <cassert> is defined in terms of ISO C, > > > and ISO C specifies that __FILE__ and __LINE__ must be used. Is that > > > another defect? But maybe the difference is not observable because > > > __FILE__ cannot be redefined? > > > > I don't think it's observable. C requires the text printed by a failed > > assert to include "the values of the preprocessing macros __FILE__ and > > __LINE__ " but it doesn't require the actual tokens to appear in the > > definition. The built-ins do return "the values" of those macros, so > > that seems OK to me. > > is that a real fix? the token sequence is the same in different > TUs with the builtins but the actual printed path is different, > so if the linker picks a definition at random then the produced > binary is not deterministic. In practice, that only happens if you copy & paste the inline function definition so it appears in two different TUs. If the inline function is defined in a header, as in 99.9% of cases, then __FILE__ and __builtin_FILE() both give you the name of that header, and the problem never happens anyway. If you do weird things (duplicating the code so it appears in two separate TUs without being included by a common header) and the inline function isn't inlined, then yes, some calls might appear to come from the "other" definition, showing the file and line number of that definition. C'est la vie. > why is this better than just using __FILE__? The version with __FILE__ has undefined behaviour, and might not even compile in a strict implementation of C++20 modules. The version with __builtin_FILE is valid C++, so required to compile. You might still get non-deterministic output if you duplicate the code (instead of defining it in a header), but that's also possible if you have a function that prints the value of rand() in an assertion message. Valid code that has non-deterministic behaviour in weird use cases is better than code that has undefined behaviour in those cases.
On Tue, Jan 24, 2023 at 3:54 AM Jonathan Wakely <jwakely@redhat.com> wrote: > Added to GCC 4.8.0 in 2012. I don't know about Clang. (Like I said in the original email.) Clang implementation is quite a bit more recent: commit 708afb56c125ca4f7db7070e836860076c7eafbc Author: Eric Fiselier <eric@efcs.ca> Date: Thu May 16 21:04:15 2019 +0000 Implement __builtin_LINE() et. al. to support source location capture. I feel like a __has_builtin gate isn't really necessary here, but will put one in if people think it's necessary. On Tue, 24 Jan 2023 at 11:38, Rich Felker <dalias@libc.org> wrote: > (Also, C++ programmers should stop putting so much code in inline > functions in headers. There are C++ projects (Boost, Eigen) which consist _entirely_ of headers. It was Eigen that has triggered this thread. We could rewrite eigen_assert() to use source_location instead of assert(), but I thought fixing it here is better, since it fixes all C++ projects and not just Eigen. -- Paul Pluzhnikov
On Tue, Jan 24, 2023 at 7:51 AM Jonathan Wakely <jwakely@redhat.com> wrote: > > On Tue, 24 Jan 2023 at 15:08, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > is that a real fix? the token sequence is the same in different > > TUs with the builtins but the actual printed path is different, > > so if the linker picks a definition at random then the produced > > binary is not deterministic. Any given binary is deterministic (the linker picks one or the other version), but rebuilding this binary again after unrelated changes may change the output, yes. > In practice, that only happens if you copy & paste the inline function > definition so it appears in two different TUs. That doesn't seem correct. As the original example shows, the same "a.h" can be #included differently in different TUs, and either __FILE__ or __builtin_FILE() will give different output in these TUs.
On Thu, 26 Jan 2023 at 19:18, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > > On Tue, Jan 24, 2023 at 7:51 AM Jonathan Wakely <jwakely@redhat.com> wrote: > > > > On Tue, 24 Jan 2023 at 15:08, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote: > > > > is that a real fix? the token sequence is the same in different > > > TUs with the builtins but the actual printed path is different, > > > so if the linker picks a definition at random then the produced > > > binary is not deterministic. > > Any given binary is deterministic (the linker picks one or the other > version), but rebuilding this binary again after unrelated changes may > change the output, yes. > > > In practice, that only happens if you copy & paste the inline function > > definition so it appears in two different TUs. > > That doesn't seem correct. > > As the original example shows, the same "a.h" can be #included > differently in different TUs, and either __FILE__ or __builtin_FILE() > will give different output in these TUs. Ah yes, agreed (sorry, I was only "invited" to the thread half way through). For __LINE__ both cases will give the same line, but the include path causes __FILE__ to be different.
Ping? Are there any strong objections to this patch which haven't been addressed? It seems that - it's a complete no-op in plain-C - it helps C++ modules - it has no real negatives and as such should be a "no brainer". Thanks,
* Paul Pluzhnikov: > Are there any strong objections to this patch which haven't been addressed? > > It seems that > - it's a complete no-op in plain-C > - it helps C++ modules > - it has no real negatives > > and as such should be a "no brainer". As I said before, use of these builtins needs to be conditionalized. Thanks, Florian
On Sun, Feb 5, 2023 at 12:08 PM Florian Weimer <fweimer@redhat.com> wrote:
> As I said before, use of these builtins needs to be conditionalized.
Yes (revised patch attached).
I was more asking whether there were any fundamental objections.
Thanks,
--
Paul Pluzhnikov
diff --git a/assert/assert.h b/assert/assert.h
index 72209bc5e7..6edcb01d0a 100644
--- a/assert/assert.h
+++ b/assert/assert.h
@@ -86,10 +86,18 @@ __END_DECLS
parentheses around EXPR. Otherwise, those added parentheses would
suppress warnings we'd expect to be detected by gcc's -Wparentheses. */
# if defined __cplusplus
+# if __has_builtin(__builtin_FILE)
+# define __ASSERT_FILE __builtin_FILE()
+# define __ASSERT_LINE __builtin_LINE()
+# else
+# define __ASSERT_FILE __FILE__
+# define __ASSERT_LINE __LINE__
+# endif
# define assert(expr) \
(static_cast <bool> (expr) \
? void (0) \
- : __assert_fail (#expr, __FILE__, __LINE__, __ASSERT_FUNCTION))
+ : __assert_fail (#expr, __ASSERT_FILE, __ASSERT_LINE, \
+ __ASSERT_FUNCTION))
# elif !defined __GNUC__ || defined __STRICT_ANSI__
# define assert(expr) \
((expr) \
* Paul Pluzhnikov: > +# if __has_builtin(__builtin_FILE) > +# define __ASSERT_FILE __builtin_FILE() > +# define __ASSERT_LINE __builtin_LINE() > +# else > +# define __ASSERT_FILE __FILE__ > +# define __ASSERT_LINE __LINE__ > +# endif __has_builtin itself needs to be conditionalized. 8-( Please use __glibc_assert_file or something like that. __ASSERT_FILE sounds like something someone else might use (although it's in the implementation namespace). I think the general direction is fine. Thanks, Florian
On Sun, Feb 5, 2023 at 2:34 PM Florian Weimer <fweimer@redhat.com> wrote: > > * Paul Pluzhnikov: > > > +# if __has_builtin(__builtin_FILE) > > +# define __ASSERT_FILE __builtin_FILE() > > +# define __ASSERT_LINE __builtin_LINE() I picked __ASSERT_FILE to match (already used) __ASSERT_FUNCTION. Mixing __glibc_assert_file with __ASSERT_FUNCTION creates a cognitive dissonance for me. How strongly do you feel about this? > __has_builtin itself needs to be conditionalized. 8-( Revised patch attached. Thanks,
* Paul Pluzhnikov: > On Sun, Feb 5, 2023 at 2:34 PM Florian Weimer <fweimer@redhat.com> wrote: >> >> * Paul Pluzhnikov: >> >> > +# if __has_builtin(__builtin_FILE) >> > +# define __ASSERT_FILE __builtin_FILE() >> > +# define __ASSERT_LINE __builtin_LINE() > > I picked __ASSERT_FILE to match (already used) __ASSERT_FUNCTION. > Mixing __glibc_assert_file with __ASSERT_FUNCTION creates a cognitive > dissonance for me. > > How strongly do you feel about this? Not strongly at all. __ASSERT_FUNCTION is probably okay. >> __has_builtin itself needs to be conditionalized. 8-( > > Revised patch attached. Missing spaces before '(', sorry. Rest looks okay if we can assume that __builtin_FILE and __builtin_LINE are implemented at the same time. Thanks, Florian
On Sun, Feb 5, 2023 at 10:01 PM Florian Weimer <fweimer@redhat.com> wrote: > Missing spaces before '(', sorry. Done. Rest looks okay if we can assume that > __builtin_FILE and __builtin_LINE are implemented at the same time. GCC has added these so long ago, we don't care (I think). LLVM commit https://reviews.llvm.org/D37035 implemented both at the same time.
On Mon, Feb 6, 2023 at 8:25 AM Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > > On Sun, Feb 5, 2023 at 10:01 PM Florian Weimer <fweimer@redhat.com> wrote: > > > Missing spaces before '(', sorry. > > Done. I intend to submit this patch on Friday if there are no additional comments. Thanks,
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) \
Greetings, Attached patch changes assert in C++ mode from using __FILE__ and __LINE__ to using __builtin_FILE() and __builtin_LINE(). These are supported by GCC since ~2012. Motivation: C++20 modules. When building C++, inline functions are required to have the exact same sequence of tokens in every translation unit. But __FILE__ token, when used in a header file, does not necessarily expand to the exact same string literal. For example: // a.h #include <assert.h> inline void fn () { assert (0); } // a.cc #include "a.h" // b.cc #include "foo/../a.h" Preprocessing a.cc and b.cc produces: (static_cast <bool> (0) ? void (0) : __assert_fail ("0", "a.h", 2, __extension__ __PRETTY_FUNCTION__)); and (static_cast <bool> (0) ? void (0) : __assert_fail ("0", "foo/../a.h", 2, __extension__ __PRETTY_FUNCTION__)); respectively. Without C++ modules, this results in unpredictable output, and technically there is an ODR violation. When C++ modules are involved, the mismatched sequence of tokens may be diagnosed and result in a build failure. P.S. I am only aware of the __FILE__ expansion causing problems, but I am changing __LINE__ as well for symmetry.