Skip to content

Commit 1f70899

Browse files
mordantecopybara-github
authored andcommitted
[libc++][module] Fixes std::string UDL. (#75000)
The fix changes the way the validation script determines the qualified name of a declaration. Inline namespaces without a reserved name are now always part of the name. The Clang code only does this when the names are ambigious. This method is often used for the operator""foo for UDLs. Adjusted the newly flagged issue and removed a work-around in the test code that is no longer required. Fixes llvm/llvm-project#72427 NOKEYCHECK=True GitOrigin-RevId: 7d34f8c09ee17327668c337ff3a7c30656f8daca
1 parent 753fbcb commit 1f70899

File tree

3 files changed

+33
-58
lines changed

3 files changed

+33
-58
lines changed

modules/std/string.inc

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,10 @@ export namespace std {
6767
// [basic.string.hash], hash support
6868
using std::hash;
6969

70-
// TODO MODULES is this a bug?
71-
#if _LIBCPP_STD_VER >= 23
72-
using std::operator""s;
73-
#else
7470
inline namespace literals {
7571
inline namespace string_literals {
7672
// [basic.string.literals], suffix for basic_string literals
7773
using std::literals::string_literals::operator""s;
7874
} // namespace string_literals
79-
} // namespace literals
80-
#endif
75+
} // namespace literals
8176
} // namespace std

test/tools/clang_tidy_checks/header_exportable_declarations.cpp

Lines changed: 32 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ void header_exportable_declarations::registerMatchers(clang::ast_matchers::Match
166166
}
167167
}
168168

169-
/// Returns the qualified name of a declaration.
169+
/// Returns the qualified name of a public declaration.
170170
///
171171
/// There is a small issue with qualified names. Typically the name returned is
172172
/// in the namespace \c std instead of the namespace \c std::__1. Except when a
@@ -182,14 +182,37 @@ void header_exportable_declarations::registerMatchers(clang::ast_matchers::Match
182182
/// * exception has equality operators for the type \c exception_ptr
183183
/// * initializer_list has the functions \c begin and \c end
184184
///
185-
/// \warning In some cases the returned name can be an empty string.
186-
/// The cause has not been investigated.
185+
/// When the named declaration uses a reserved name the result is an
186+
/// empty string.
187187
static std::string get_qualified_name(const clang::NamedDecl& decl) {
188-
std::string result = decl.getQualifiedNameAsString();
189-
190-
if (result.starts_with("std::__1::"))
191-
result.erase(5, 5);
192-
188+
std::string result = decl.getNameAsString();
189+
// Reject reserved names (ignoring _ in global namespace).
190+
if (result.size() >= 2 && result[0] == '_')
191+
if (result[1] == '_' || std::isupper(result[1]))
192+
if (result != "_Exit")
193+
return "";
194+
195+
for (auto* context = llvm::dyn_cast_or_null<clang::NamespaceDecl>(decl.getDeclContext()); //
196+
context;
197+
context = llvm::dyn_cast_or_null<clang::NamespaceDecl>(context->getDeclContext())) {
198+
std::string ns = std::string(context->getName());
199+
200+
if (ns.starts_with("__")) {
201+
// When the reserved name is an inline namespace the namespace is
202+
// not added to the qualified name instead of removed. Libc++ uses
203+
// several inline namespace with reserved names. For example,
204+
// __1 for every declaration, __cpo in range-based algorithms.
205+
//
206+
// Note other inline namespaces are expanded. This resolves
207+
// ambiguity when two named declarations have the same name but in
208+
// different inline namespaces. These typically are the literal
209+
// conversion operators like operator""s which can be a
210+
// std::string or std::chrono::seconds.
211+
if (!context->isInline())
212+
return "";
213+
} else
214+
result = ns + "::" + result;
215+
}
193216
return result;
194217
}
195218

@@ -220,38 +243,6 @@ static bool is_viable_declaration(const clang::NamedDecl* decl) {
220243
return llvm::isa<clang::EnumDecl, clang::VarDecl, clang::ConceptDecl, clang::TypedefNameDecl, clang::UsingDecl>(decl);
221244
}
222245

223-
/// Returns the name is a reserved name.
224-
///
225-
/// Detected reserved names are names starting with __ or _[A-Z].
226-
/// These names can be in the global namespace, std namespace or any namespace
227-
/// inside std. For example, std::ranges contains reserved names to implement
228-
/// the Niebloids.
229-
///
230-
/// This test misses candidates which are not used in libc++
231-
/// * any identifier with two underscores not at the start
232-
bool is_reserved_name(std::string_view name) {
233-
if (name.starts_with("_")) {
234-
// This is a public name declared in cstdlib.
235-
if (name == "_Exit")
236-
return false;
237-
238-
return name.size() > 1 && (name[1] == '_' || std::isupper(name[1]));
239-
}
240-
241-
std::size_t pos = name.find("::_");
242-
if (pos == std::string::npos)
243-
return false;
244-
245-
if (pos + 3 > name.size())
246-
return false;
247-
248-
// This is a public name declared in cstdlib.
249-
if (name == "std::_Exit")
250-
return false;
251-
252-
return name[pos + 3] == '_' || std::isupper(name[pos + 3]);
253-
}
254-
255246
/// Some declarations in the global namespace are exported from the std module.
256247
static bool is_global_name_exported_by_std_module(std::string_view name) {
257248
static const std::set<std::string_view> valid{
@@ -297,9 +288,6 @@ void header_exportable_declarations::check(const clang::ast_matchers::MatchFinde
297288
if (name.empty())
298289
return;
299290

300-
if (is_reserved_name(name))
301-
return;
302-
303291
// For modules only take the declarations exported.
304292
if (is_module(file_type_))
305293
if (decl->getModuleOwnershipKind() != clang::Decl::ModuleOwnershipKind::VisibleWhenImported)
@@ -336,7 +324,7 @@ void header_exportable_declarations::check(const clang::ast_matchers::MatchFinde
336324
return;
337325

338326
std::string name = get_qualified_name(*decl);
339-
if (is_reserved_name(name))
327+
if (name.empty())
340328
return;
341329

342330
if (global_decls_.contains(name))

utils/libcxx/test/modules.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,6 @@ def process_module_partition(self, header, is_c_header):
141141
f"# include <{header}>{nl}"
142142
f"#endif{nl}"
143143
)
144-
elif header == "chrono":
145-
# When localization is disabled the header string is not included.
146-
# When string is included chrono's operator""s is a named declaration
147-
# using std::chrono_literals::operator""s;
148-
# else it is a named declaration
149-
# using std::operator""s;
150-
# TODO MODULES investigate why
151-
include = f"#include <string>{nl}#include <chrono>{nl}"
152144
else:
153145
include = f"#include <{header}>{nl}"
154146

0 commit comments

Comments
 (0)