[RFC][clang][Support] Extract type parsing function. [NFCI] #82797
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang Author: Francesco Petrogalli (fpetrogalli) ChangesI wanted to get a sense if people are OK for me to provide unit tests in The refactoring would look like this patch - I haven't added any actual tests because I didn't want to keep working on this if people have strong arguments against it. FWIW, I believe that this refactoring will make it easier to handle new types over time. Pleas note that this is an RFC - I am not asking for a fully detailed review, just for some feedback on the idea. Full diff: https://github.com/llvm/llvm-project/pull/82797.diff 4 Files Affected:
diff --git a/clang/include/clang/Support/BuiltinsUtils.h b/clang/include/clang/Support/BuiltinsUtils.h
new file mode 100644
index 00000000000000..2ee26f28127ce9
--- /dev/null
+++ b/clang/include/clang/Support/BuiltinsUtils.h
@@ -0,0 +1,24 @@
+//===--- BuiltinsUtils.h - clang Builtins Utils -*- C++ -*-----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef CLANG_SUPPORT_BUILTINSUTILS_H
+#define CLANG_SUPPORT_BUILTINSUTILS_H
+
+#include "llvm/ADT/StringRef.h"
+#include <string>
+namespace llvm {
+class SMLoc;
+}
+namespace clang {
+
+/// Parse builtins prototypes according to the rules in
+/// clang/include/clang/Basic/Builtins.def
+void ParseBuiltinType(llvm::StringRef T, llvm::StringRef Substitution,
+ std::string &Type, llvm::SMLoc *Loc);
+
+} // namespace clang
+#endif // CLANG_SUPPORT_BUILTINSUTILS_H
diff --git a/clang/lib/Support/CMakeLists.txt b/clang/lib/Support/CMakeLists.txt
index 8ea5620052ed84..93959eb8565b6b 100644
--- a/clang/lib/Support/CMakeLists.txt
+++ b/clang/lib/Support/CMakeLists.txt
@@ -11,6 +11,7 @@ set(LLVM_LINK_COMPONENTS
set(clangSupport_sources
RISCVVIntrinsicUtils.cpp
+ ClangBuiltinsUtils.cpp
)
add_clang_library(clangSupport ${clangSupport_sources})
diff --git a/clang/lib/Support/ClangBuiltinsUtils.cpp b/clang/lib/Support/ClangBuiltinsUtils.cpp
new file mode 100644
index 00000000000000..523378322cdf1b
--- /dev/null
+++ b/clang/lib/Support/ClangBuiltinsUtils.cpp
@@ -0,0 +1,81 @@
+#include "clang/Support/BuiltinsUtils.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/SMLoc.h"
+#include "llvm/TableGen/Error.h"
+
+void clang::ParseBuiltinType(llvm::StringRef T, llvm::StringRef Substitution,
+ std::string &Type, llvm::SMLoc *Loc) {
+ assert(Loc);
+ T = T.trim();
+ if (T.consume_back("*")) {
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ Type += "*";
+ } else if (T.consume_back("const")) {
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ Type += "C";
+ } else if (T.consume_back("volatile")) {
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ Type += "D";
+ } else if (T.consume_back("restrict")) {
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ Type += "R";
+ } else if (T.consume_back("&")) {
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ Type += "&";
+ } else if (T.consume_front("long")) {
+ Type += "L";
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ } else if (T.consume_front("unsigned")) {
+ Type += "U";
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ } else if (T.consume_front("_Complex")) {
+ Type += "X";
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ } else if (T.consume_front("_Constant")) {
+ Type += "I";
+ ParseBuiltinType(T, Substitution, Type, Loc);
+ } else if (T.consume_front("T")) {
+ if (Substitution.empty())
+ llvm::PrintFatalError(*Loc, "Not a template");
+ ParseBuiltinType(Substitution, Substitution, Type, Loc);
+ } else {
+ auto ReturnTypeVal = llvm::StringSwitch<std::string>(T)
+ .Case("__builtin_va_list_ref", "A")
+ .Case("__builtin_va_list", "a")
+ .Case("__float128", "LLd")
+ .Case("__fp16", "h")
+ .Case("__int128_t", "LLLi")
+ .Case("_Float16", "x")
+ .Case("bool", "b")
+ .Case("char", "c")
+ .Case("constant_CFString", "F")
+ .Case("double", "d")
+ .Case("FILE", "P")
+ .Case("float", "f")
+ .Case("id", "G")
+ .Case("int", "i")
+ .Case("int32_t", "Zi")
+ .Case("int64_t", "Wi")
+ .Case("jmp_buf", "J")
+ .Case("msint32_t", "Ni")
+ .Case("msuint32_t", "UNi")
+ .Case("objc_super", "M")
+ .Case("pid_t", "p")
+ .Case("ptrdiff_t", "Y")
+ .Case("SEL", "H")
+ .Case("short", "s")
+ .Case("sigjmp_buf", "SJ")
+ .Case("size_t", "z")
+ .Case("ucontext_t", "K")
+ .Case("uint32_t", "UZi")
+ .Case("uint64_t", "UWi")
+ .Case("void", "v")
+ .Case("wchar_t", "w")
+ .Case("...", ".")
+ .Default("error");
+ if (ReturnTypeVal == "error")
+ llvm::PrintFatalError(*Loc, "Unknown Type: " + T);
+
+ Type += ReturnTypeVal;
+ }
+}
diff --git a/clang/utils/TableGen/ClangBuiltinsEmitter.cpp b/clang/utils/TableGen/ClangBuiltinsEmitter.cpp
index 48f55b8af97e4e..27c47335a68c58 100644
--- a/clang/utils/TableGen/ClangBuiltinsEmitter.cpp
+++ b/clang/utils/TableGen/ClangBuiltinsEmitter.cpp
@@ -11,11 +11,11 @@
//===----------------------------------------------------------------------===//
#include "TableGenBackends.h"
+#include "clang/Support/BuiltinsUtils.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/TableGen/Error.h"
#include "llvm/TableGen/Record.h"
#include "llvm/TableGen/TableGenBackend.h"
-
using namespace llvm;
namespace {
@@ -53,77 +53,7 @@ class PrototypeParser {
}
void ParseType(StringRef T) {
- T = T.trim();
- if (T.consume_back("*")) {
- ParseType(T);
- Type += "*";
- } else if (T.consume_back("const")) {
- ParseType(T);
- Type += "C";
- } else if (T.consume_back("volatile")) {
- ParseType(T);
- Type += "D";
- } else if (T.consume_back("restrict")) {
- ParseType(T);
- Type += "R";
- } else if (T.consume_back("&")) {
- ParseType(T);
- Type += "&";
- } else if (T.consume_front("long")) {
- Type += "L";
- ParseType(T);
- } else if (T.consume_front("unsigned")) {
- Type += "U";
- ParseType(T);
- } else if (T.consume_front("_Complex")) {
- Type += "X";
- ParseType(T);
- } else if (T.consume_front("_Constant")) {
- Type += "I";
- ParseType(T);
- } else if (T.consume_front("T")) {
- if (Substitution.empty())
- PrintFatalError(Loc, "Not a template");
- ParseType(Substitution);
- } else {
- auto ReturnTypeVal = StringSwitch<std::string>(T)
- .Case("__builtin_va_list_ref", "A")
- .Case("__builtin_va_list", "a")
- .Case("__float128", "LLd")
- .Case("__fp16", "h")
- .Case("__int128_t", "LLLi")
- .Case("_Float16", "x")
- .Case("bool", "b")
- .Case("char", "c")
- .Case("constant_CFString", "F")
- .Case("double", "d")
- .Case("FILE", "P")
- .Case("float", "f")
- .Case("id", "G")
- .Case("int", "i")
- .Case("int32_t", "Zi")
- .Case("int64_t", "Wi")
- .Case("jmp_buf", "J")
- .Case("msint32_t", "Ni")
- .Case("msuint32_t", "UNi")
- .Case("objc_super", "M")
- .Case("pid_t", "p")
- .Case("ptrdiff_t", "Y")
- .Case("SEL", "H")
- .Case("short", "s")
- .Case("sigjmp_buf", "SJ")
- .Case("size_t", "z")
- .Case("ucontext_t", "K")
- .Case("uint32_t", "UZi")
- .Case("uint64_t", "UWi")
- .Case("void", "v")
- .Case("wchar_t", "w")
- .Case("...", ".")
- .Default("error");
- if (ReturnTypeVal == "error")
- PrintFatalError(Loc, "Unknown Type: " + T);
- Type += ReturnTypeVal;
- }
+ clang::ParseBuiltinType(T, Substitution, Type, &Loc);
}
public:
|
fpetrogalli
requested review from
michele-scandale,
AaronBallman and
tbaederr
|
I have added for feedback all people involved in #68324 |
fpetrogalli
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I wanted to get a sense if people are OK for me to provide unit tests in
clang/unittests/Supportfor the the parsing function I factored out fromclang-tblgenintoclangSupport.The refactoring would look like this patch - I haven't added any actual tests because I didn't want to keep working on this if people have strong arguments against it.
FWIW, I believe that this refactoring will make it easier to handle new types over time.
Pleas note that this is an RFC - I am not asking for a fully detailed review, just for some feedback on the idea.