RFC: [llvm] specialize cl::opt_storage for std::string #149172
Closed
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.
Purpose
Specialize
llvm::cl::opt_storagefor data typestd::stringso that it no longer inherits fromstd::string. With this patch in place, explicitly instantiatedcl::opt<std::string>can be safely exported from an LLVM Windows DLL.Overview
cl::opt_storagefor data typestd::string. It does NOT inherit fromstd::stringas it did previously when data typestd::stringmatched the partially specified templatecl::opt_storage<DataType, false, true>std::string,llvm::StringRef, andllvm::Twineso that instances can be used in many places wherestd::stringis used.std::stringmethods so that instances of the class are mostly interoperable withstd::string.Tripleso it properly constructs fromcl::opt_storage<std::string>. This change avoids having to modify clients that constructTriples fromcl::opt<std::string>instances.Background
This patch is in support of annotating LLVM's public symbols for Windows DLL export., tracked in #109483. Additional context is provided in this discourse.
This change is needed because, without it, we cannot export
opt<std::string>from an LLVM Windows DLL. This is because MSVC exports all ancestor classes when exporting an instantiated template class. Since one ofopt's ancestor classes is its type argument (viaopt_storage), it is an ancestor ofstd::string. Therefore, if we exportopt<std::string>from the LLVM DLL, MSVC forcesstd::basic_stringto also be exported. This leads to duplicate symbol errors and generally seems like a bad idea. Compiling withclang-cldoes not exhibit this behavior.Validation
clandclang-cl.clangandgcc.