Skip to content

Conversation

@vinay-deshmukh
Copy link
Contributor

@vinay-deshmukh vinay-deshmukh commented Mar 6, 2025

Fixes #15522

Essentially all of math.h doesn't exist until C99: https://en.cppreference.com/w/c/numeric/math/log

Todo: need more granular filtering based on poring the reference

@frederick-vs-ja
Copy link
Contributor

Essentially all of math.h doesn't exist until C99: https://en.cppreference.com/w/c/numeric/math/log

You seemed to misread the page. logf and logl exist since C99, while log exists since C89.

@vinay-deshmukh
Copy link
Contributor Author

Essentially all of math.h doesn't exist until C99: https://en.cppreference.com/w/c/numeric/math/log

You seemed to misread the page. logf and logl exist since C99, while log exists since C89.

@frederick-vs-ja

My bad, I should have better clarified my previous comment.

While log does exist since C89 within <math.h>, it should be allowed for a .c to define int log = 0; because the math.h has not been included. Thereby, even if double log(double); exists in C89 (in math.h), it's not been declared in the current translation unit, so declaring a symbol with the same name as log seems to be valid IMO.

GCC's behavior seems to be that even if builtins are "re-defined" it is allowed, when there is no explicit include for math.h

Reference:

I've made a godbolt link https://godbolt.org/z/zY76dWa84 with all the symbols on https://en.cppreference.com/w/c/numeric/math

All panels have -std=c89 in common and are the respective trunk versions.

  1. Panel 1: GCC builds with warnings [-Wbuiltin-declaration-mismatch]
  2. Panel 2: Clang builds (after passing non-default options, this PR will make those effective when using -std=c89) and no warnings despite -Wall
  3. Panel 3: Clang fails to build (this issue)

@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Mar 7, 2025

While log does exist since C89 within <math.h>, it should be allowed for a .c to define int log = 0; because the math.h has not been included. Thereby, even if double log(double); exists in C89 (in math.h), it's not been declared in the current translation unit, so declaring a symbol with the same name as log seems to be valid IMO.

However, declaration it as an object with external linkage is UB (or ill-formed, no diagnostic required in the C++ standard wording).

Per N3220 7.1.3/1

[...]
— All identifiers with external linkage in any of the following subclauses (including the future library directions) and errno are always reserved for use as identifiers with external linkage.218)
[...]

In C89 that was in 4.1.2

[...] All external identifiers declared in any of the headers are reserved, whether or not the associated header is included. [...]

It seem to me that the following program was well-defined in C89 but became undefined (IFNDR) since C99.

int logf = 0; /* N.B. static not used */
int main(void) { return logf; }

But if logf is replaced with log, the program is invalid even in C89.

@github-actions
Copy link

github-actions bot commented Mar 9, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@vinay-deshmukh
Copy link
Contributor Author

I'm a bit confused about the whole what is not UB/IFNDR in C89, but is in C99. Based on my current understanding the simplest way to resolve the issue seems to what I've done as of:
7572c21

Is not define builtins for symbols available in C89?

let me know if that looks good.

@vinay-deshmukh vinay-deshmukh force-pushed the vinay-issue-15522-c89 branch from d79f127 to 6b05ad8 Compare March 10, 2025 01:46
This reverts commit 6b05ad8.
@vinay-deshmukh vinay-deshmukh force-pushed the vinay-issue-15522-c89 branch from fda5122 to 951bff3 Compare March 10, 2025 02:01
@vinay-deshmukh vinay-deshmukh force-pushed the vinay-issue-15522-c89 branch from 709b617 to 9a6d3a8 Compare March 10, 2025 02:41
@vinay-deshmukh vinay-deshmukh changed the title C89 doesn't have math.h functions Fixes: Can not use C99 function names as variable names in C89 Mar 10, 2025
@vinay-deshmukh
Copy link
Contributor Author

@frederick-vs-ja

Any suggestions on how to go about making further progress on this one? Sort of stuck right now.

@vinay-deshmukh
Copy link
Contributor Author

@frederick-vs-ja

Any suggestions on how to go about making further progress on this one? Sort of stuck right now.

@frederick-vs-ja bump

Comment on lines +150 to +153
if (NameStr == "log" || NameStr == "va_start" || NameStr == "va_arg" ||
NameStr == "va_end") {
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list would be very long, see https://port70.net/~nsz/c/c89/c89-draft.html. Perhaps we should add a .def file for this.

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.

Can not use C99 function names as variable names in C89

2 participants