[clang] C89 doesn't have `math.h` functions (PR #129979)
Vinay Deshmukh via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 9 18:57:52 PDT 2025
https://github.com/vinay-deshmukh updated https://github.com/llvm/llvm-project/pull/129979
>From e9c7869550d9fd1eba4d4d42ee644540e6b6d445 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Tue, 4 Mar 2025 22:30:34 -0500
Subject: [PATCH 1/9] add failing test
---
clang/test/C/drs/c89_with_c99_functions.c | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 clang/test/C/drs/c89_with_c99_functions.c
diff --git a/clang/test/C/drs/c89_with_c99_functions.c b/clang/test/C/drs/c89_with_c99_functions.c
new file mode 100644
index 0000000000000..d848727001562
--- /dev/null
+++ b/clang/test/C/drs/c89_with_c99_functions.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c89 -verify %s
+
+// From: https://github.com/llvm/llvm-project/issues/15522#issue-1071059939
+int logf = 5;
+int main() {
+return logf;
+}
>From 966aa6a735722063d7ea9727e36136f4f39c3d88 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Wed, 5 Mar 2025 22:09:02 -0500
Subject: [PATCH 2/9] Update test to `expect-no-diagnostics`
---
clang/test/C/drs/c89_with_c99_functions.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/test/C/drs/c89_with_c99_functions.c b/clang/test/C/drs/c89_with_c99_functions.c
index d848727001562..de525313fbba5 100644
--- a/clang/test/C/drs/c89_with_c99_functions.c
+++ b/clang/test/C/drs/c89_with_c99_functions.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -std=c89 -verify %s
+// expected-no-diagnostics
// From: https://github.com/llvm/llvm-project/issues/15522#issue-1071059939
int logf = 5;
>From 28c58718d76be9c62635cdeffebc9d812b192e50 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Wed, 5 Mar 2025 22:13:50 -0500
Subject: [PATCH 3/9] Add exclusion for C89 / math.h functions
---
clang/lib/Basic/Builtins.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index e7829a461bbc5..cfcbefdce2a24 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -14,6 +14,7 @@
#include "BuiltinTargetFeatures.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/LangStandard.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringRef.h"
using namespace clang;
@@ -148,7 +149,7 @@ static bool builtinIsSupported(const llvm::StringTable &Strings,
if (!LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG))
return false;
/* MathBuiltins Unsupported */
- if (LangOpts.NoMathBuiltin && BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
+ if ((LangOpts.NoMathBuiltin || /*C89*/ LangOpts.LangStd == LangStandard::lang_c89)&& BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
return false;
/* GnuMode Unsupported */
if (!LangOpts.GNUMode && (BuiltinInfo.Langs & GNU_LANG))
>From 6e790580b3859cd68ca6ea87c1601979047310ab Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Wed, 5 Mar 2025 22:25:07 -0500
Subject: [PATCH 4/9] clang-format
---
clang/lib/Basic/Builtins.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index cfcbefdce2a24..81f85bb5454e4 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -149,7 +149,9 @@ static bool builtinIsSupported(const llvm::StringTable &Strings,
if (!LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG))
return false;
/* MathBuiltins Unsupported */
- if ((LangOpts.NoMathBuiltin || /*C89*/ LangOpts.LangStd == LangStandard::lang_c89)&& BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
+ if ((LangOpts.NoMathBuiltin ||
+ /*C89*/ LangOpts.LangStd == LangStandard::lang_c89) &&
+ BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
return false;
/* GnuMode Unsupported */
if (!LangOpts.GNUMode && (BuiltinInfo.Langs & GNU_LANG))
>From 160d719b7ed5997d39accb110df873e6200991d4 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sun, 9 Mar 2025 14:14:37 -0400
Subject: [PATCH 5/9] exclude c99 symbols from builtins when std=c89
---
clang/lib/Basic/Builtins.cpp | 20 ++++++++++++++++++--
clang/test/C/drs/c89_with_c99_functions.c | 14 ++++++++++----
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index 81f85bb5454e4..70ea34da1f9e6 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -17,6 +17,7 @@
#include "clang/Basic/LangStandard.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringTable.h"
using namespace clang;
const char *HeaderDesc::getName() const {
@@ -136,6 +137,18 @@ bool Builtin::Context::isBuiltinFunc(llvm::StringRef FuncName) {
return false;
}
+static bool isSymbolAvailableInC89(const llvm::StringTable& Strings, const Builtin::Info & BuiltinInfo) {
+
+ auto NameStr = Strings[BuiltinInfo.Offsets.Name];
+
+ // FIXME: add other C89 symbols here
+ if(NameStr == "log") {
+ return true;
+ }
+
+ return false;
+}
+
/// Is this builtin supported according to the given language options?
static bool builtinIsSupported(const llvm::StringTable &Strings,
const Builtin::Info &BuiltinInfo,
@@ -148,9 +161,12 @@ static bool builtinIsSupported(const llvm::StringTable &Strings,
/* CorBuiltins Unsupported */
if (!LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG))
return false;
+ bool isC89 = /*C89*/ LangOpts.LangStd == LangStandard::lang_c89;
+ if (isC89 && !isSymbolAvailableInC89(Strings, BuiltinInfo)) {
+ return false;
+ }
/* MathBuiltins Unsupported */
- if ((LangOpts.NoMathBuiltin ||
- /*C89*/ LangOpts.LangStd == LangStandard::lang_c89) &&
+ if (LangOpts.NoMathBuiltin &&
BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
return false;
/* GnuMode Unsupported */
diff --git a/clang/test/C/drs/c89_with_c99_functions.c b/clang/test/C/drs/c89_with_c99_functions.c
index de525313fbba5..aa3672eda1c53 100644
--- a/clang/test/C/drs/c89_with_c99_functions.c
+++ b/clang/test/C/drs/c89_with_c99_functions.c
@@ -1,8 +1,14 @@
-// RUN: %clang_cc1 -std=c89 -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -std=c89 -verify=c89 %s
// From: https://github.com/llvm/llvm-project/issues/15522#issue-1071059939
-int logf = 5;
+int logf = 5; // this is fine
+
+// redefinition because log as a symbol exists in C89
+int log = 6; // #1
int main() {
-return logf;
+return 0;
}
+
+// c89-error@#1 {{redefinition of 'log' as different kind of symbol}}
+// c89-note@#1 {{unguarded header; consider using #ifdef guards or #pragma once}}
+// c89-note@#1 {{previous definition}}
>From 7572c217f84214af8aa06a219ead9382af6b148b Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sun, 9 Mar 2025 14:19:20 -0400
Subject: [PATCH 6/9] clang-format
---
clang/lib/Basic/Builtins.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index 70ea34da1f9e6..4795a7fca6a08 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -137,12 +137,13 @@ bool Builtin::Context::isBuiltinFunc(llvm::StringRef FuncName) {
return false;
}
-static bool isSymbolAvailableInC89(const llvm::StringTable& Strings, const Builtin::Info & BuiltinInfo) {
+static bool isSymbolAvailableInC89(const llvm::StringTable &Strings,
+ const Builtin::Info &BuiltinInfo) {
auto NameStr = Strings[BuiltinInfo.Offsets.Name];
// FIXME: add other C89 symbols here
- if(NameStr == "log") {
+ if (NameStr == "log") {
return true;
}
@@ -166,8 +167,7 @@ static bool builtinIsSupported(const llvm::StringTable &Strings,
return false;
}
/* MathBuiltins Unsupported */
- if (LangOpts.NoMathBuiltin &&
- BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
+ if (LangOpts.NoMathBuiltin && BuiltinInfo.Header.ID == HeaderDesc::MATH_H)
return false;
/* GnuMode Unsupported */
if (!LangOpts.GNUMode && (BuiltinInfo.Langs & GNU_LANG))
>From 0afb18c710d8f6d5fb14ed62d0a003fdd9a44631 Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sun, 9 Mar 2025 21:26:19 -0400
Subject: [PATCH 7/9] Allow __builtin_ prefixes
---
clang/lib/Basic/Builtins.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index 4795a7fca6a08..5bc5270e248fb 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -142,6 +142,10 @@ static bool isSymbolAvailableInC89(const llvm::StringTable &Strings,
auto NameStr = Strings[BuiltinInfo.Offsets.Name];
+ if(NameStr.starts_with("__builtin_")) {
+ return true;
+ }
+
// FIXME: add other C89 symbols here
if (NameStr == "log") {
return true;
>From 6b05ad872c3959dc64ce1b0518686dad6725be8d Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sun, 9 Mar 2025 21:37:10 -0400
Subject: [PATCH 8/9] __clang_
---
clang/lib/Basic/Builtins.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index 5bc5270e248fb..cf482fd000212 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -142,7 +142,7 @@ static bool isSymbolAvailableInC89(const llvm::StringTable &Strings,
auto NameStr = Strings[BuiltinInfo.Offsets.Name];
- if(NameStr.starts_with("__builtin_")) {
+ if (NameStr.starts_with("__builtin_") || NameStr.starts_with("__clang")) {
return true;
}
>From fda5122778a2553706b1a4f953c04d45bfaf332e Mon Sep 17 00:00:00 2001
From: Vinay Deshmukh <32487576+vinay-deshmukh at users.noreply.github.com>
Date: Sun, 9 Mar 2025 21:57:22 -0400
Subject: [PATCH 9/9] Revert "__clang_"
This reverts commit 6b05ad872c3959dc64ce1b0518686dad6725be8d.
---
clang/lib/Basic/Builtins.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index cf482fd000212..5bc5270e248fb 100644
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -142,7 +142,7 @@ static bool isSymbolAvailableInC89(const llvm::StringTable &Strings,
auto NameStr = Strings[BuiltinInfo.Offsets.Name];
- if (NameStr.starts_with("__builtin_") || NameStr.starts_with("__clang")) {
+ if(NameStr.starts_with("__builtin_")) {
return true;
}
More information about the cfe-commits
mailing list