Revert "[Clang] Take libstdc++ into account during GCC detection" #154368
Conversation
This file contains hidden or 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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Frederik Harwath (frederik-h) ChangesReverts llvm/llvm-project#145056 Patch is 41.01 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/154368.diff 37 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8f80b4cd49bd7..a8dc5352fcbb4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -37,22 +37,6 @@ latest release, please see the `Clang Web Site <https://clang.llvm.org>`_ or the
Potentially Breaking Changes
============================
-- Clang will now emit a warning if the auto-detected GCC installation
- directory (i.e. the one with the largest version number) does not
- contain libstdc++ include directories although a "complete" GCC
- installation directory containing the include directories is
- available. It is planned to change the auto-detection to prefer the
- "complete" directory in the future. The warning will disappear if
- the libstdc++ include directories are either installed or removed
- for all GCC installation directories considered by the
- auto-detection; see the output of ``clang -v`` for a list of those
- directories. If the GCC installations cannot be modified and
- maintaining the current choice of the auto-detection is desired, the
- GCC installation directory can be selected explicitly using the
- ``--gcc-install-dir`` command line argument. This will silence the
- warning. It can also be disabled using the
- ``-Wno-gcc-install-dir-libstdcxx`` command line flag.
-
C/C++ Language Potentially Breaking Changes
-------------------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index b8c7c6e8d6909..6df8f9932f30f 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -885,9 +885,4 @@ def warn_drv_openacc_without_cir
: Warning<"OpenACC directives will result in no runtime behavior; use "
"-fclangir to enable runtime effect">,
InGroup<SourceUsesOpenACC>;
-
-def warn_drv_gcc_install_dir_libstdcxx : Warning<
- "future releases of the clang compiler will prefer GCC installations "
- "containing libstdc++ include directories; '%0' would be chosen over '%1'">,
- InGroup<DiagGroup<"gcc-install-dir-libstdcxx">>;
}
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 1425714d34110..243056360370f 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -224,6 +224,9 @@ class ToolChain {
static void addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
+ static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ const Twine &Path);
static void addExternCSystemInclude(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args,
const Twine &Path);
@@ -243,9 +246,6 @@ class ToolChain {
///@}
public:
- static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- const Twine &Path);
virtual ~ToolChain();
// Accessors
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 65b36217a940f..7667dbddb0ca2 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1409,6 +1409,13 @@ void ToolChain::addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
CC1Args.push_back(DriverArgs.MakeArgString(Path));
}
+/// Utility function to add a system include directory to CC1 arguments.
+void ToolChain::addSystemInclude(const ArgList &DriverArgs,
+ ArgStringList &CC1Args, const Twine &Path) {
+ CC1Args.push_back("-internal-isystem");
+ CC1Args.push_back(DriverArgs.MakeArgString(Path));
+}
+
/// Utility function to add a system include directory with extern "C"
/// semantics to CC1 arguments.
///
@@ -1431,14 +1438,6 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
addExternCSystemInclude(DriverArgs, CC1Args, Path);
}
-/// Utility function to add a system include directory to CC1 arguments.
-/*static*/ void ToolChain::addSystemInclude(const ArgList &DriverArgs,
- ArgStringList &CC1Args,
- const Twine &Path) {
- CC1Args.push_back("-internal-isystem");
- CC1Args.push_back(DriverArgs.MakeArgString(Path));
-}
-
/// Utility function to add a list of system framework directories to CC1.
void ToolChain::addSystemFrameworkIncludes(const ArgList &DriverArgs,
ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 3dade2bdf2277..01b146db24f3e 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2123,11 +2123,10 @@ void Generic_GCC::GCCInstallationDetector::init(
StringRef TripleText =
llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
- SelectedInstallation.Version = GCCVersion::Parse(VersionText);
- SelectedInstallation.GCCTriple.setTriple(TripleText);
- SelectedInstallation.GCCInstallPath = std::string(InstallDir);
- SelectedInstallation.GCCParentLibPath =
- SelectedInstallation.GCCInstallPath + "/../../..";
+ Version = GCCVersion::Parse(VersionText);
+ GCCTriple.setTriple(TripleText);
+ GCCInstallPath = std::string(InstallDir);
+ GCCParentLibPath = GCCInstallPath + "/../../..";
IsValid = true;
}
return;
@@ -2187,7 +2186,7 @@ void Generic_GCC::GCCInstallationDetector::init(
// Loop over the various components which exist and select the best GCC
// installation available. GCC installs are ranked by version number.
const GCCVersion VersionZero = GCCVersion::Parse("0.0.0");
- SelectedInstallation.Version = VersionZero;
+ Version = VersionZero;
for (const std::string &Prefix : Prefixes) {
auto &VFS = D.getVFS();
if (!VFS.exists(Prefix))
@@ -2215,7 +2214,7 @@ void Generic_GCC::GCCInstallationDetector::init(
}
// Skip other prefixes once a GCC installation is found.
- if (SelectedInstallation.Version > VersionZero)
+ if (Version > VersionZero)
break;
}
}
@@ -2224,17 +2223,14 @@ void Generic_GCC::GCCInstallationDetector::print(raw_ostream &OS) const {
for (const auto &InstallPath : CandidateGCCInstallPaths)
OS << "Found candidate GCC installation: " << InstallPath << "\n";
- if (!SelectedInstallation.GCCInstallPath.empty())
- OS << "Selected GCC installation: " << SelectedInstallation.GCCInstallPath
- << "\n";
+ if (!GCCInstallPath.empty())
+ OS << "Selected GCC installation: " << GCCInstallPath << "\n";
for (const auto &Multilib : Multilibs)
OS << "Candidate multilib: " << Multilib << "\n";
- if (Multilibs.size() != 0 ||
- !SelectedInstallation.SelectedMultilib.isDefault())
- OS << "Selected multilib: " << SelectedInstallation.SelectedMultilib
- << "\n";
+ if (Multilibs.size() != 0 || !SelectedMultilib.isDefault())
+ OS << "Selected multilib: " << SelectedMultilib << "\n";
}
bool Generic_GCC::GCCInstallationDetector::getBiarchSibling(Multilib &M) const {
@@ -2772,50 +2768,14 @@ bool Generic_GCC::GCCInstallationDetector::ScanGCCForMultilibs(
}
Multilibs = Detected.Multilibs;
- SelectedInstallation.SelectedMultilib =
- Detected.SelectedMultilibs.empty() ? Multilib()
- : Detected.SelectedMultilibs.back();
+ SelectedMultilib = Detected.SelectedMultilibs.empty()
+ ? Multilib()
+ : Detected.SelectedMultilibs.back();
BiarchSibling = Detected.BiarchSibling;
return true;
}
-bool Generic_GCC::GCCInstallationDetector::SelectGCCInstallationDirectory(
- const SmallVector<Generic_GCC::GCCInstallCandidate, 3> &Installations,
- const ArgList &Args,
- Generic_GCC::GCCInstallCandidate &SelectedInstallation) const {
- if (Installations.empty())
- return false;
-
- SelectedInstallation =
- *max_element(Installations, [](const auto &Max, const auto &I) {
- return I.Version > Max.Version;
- });
-
- // FIXME Start selecting installation with libstdc++ in clang 22,
- // using the current way of selecting the installation as a fallback
- // only. For now, warn if the installation with libstdc++ differs
- // from SelectedInstallation.
- const GCCInstallCandidate *InstallWithIncludes = nullptr;
- for (const auto &I : Installations) {
- if ((!InstallWithIncludes || I.Version > InstallWithIncludes->Version) &&
- GCCInstallationHasLibStdcxxIncludePaths(I, Args))
- InstallWithIncludes = &I;
- }
-
- if (InstallWithIncludes && SelectedInstallation.GCCInstallPath !=
- InstallWithIncludes->GCCInstallPath)
- D.Diag(diag::warn_drv_gcc_install_dir_libstdcxx)
- << InstallWithIncludes->GCCInstallPath
- << SelectedInstallation.GCCInstallPath;
-
- // TODO Warn if SelectedInstallation does not contain libstdc++ includes
- // although compiler flags indicate that it is required (C++ compilation,
- // libstdc++ not explicitly disabled).
-
- return true;
-}
-
void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
const llvm::Triple &TargetTriple, const ArgList &Args,
const std::string &LibDir, StringRef CandidateTriple,
@@ -2845,7 +2805,6 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
TargetTriple.getVendor() == llvm::Triple::Freescale ||
TargetTriple.getVendor() == llvm::Triple::OpenEmbedded}};
- SmallVector<GCCInstallCandidate, 3> Installations;
for (auto &Suffix : Suffixes) {
if (!Suffix.Active)
continue;
@@ -2863,31 +2822,23 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple(
continue; // Saw this path before; no need to look at it again.
if (CandidateVersion.isOlderThan(4, 1, 1))
continue;
- if (CandidateVersion <= SelectedInstallation.Version && IsValid)
+ if (CandidateVersion <= Version)
continue;
if (!ScanGCCForMultilibs(TargetTriple, Args, LI->path(),
NeedsBiarchSuffix))
continue;
- GCCInstallCandidate Installation;
- Installation.Version = CandidateVersion;
- Installation.GCCTriple.setTriple(CandidateTriple);
+ Version = CandidateVersion;
+ GCCTriple.setTriple(CandidateTriple);
// FIXME: We hack together the directory name here instead of
// using LI to ensure stable path separators across Windows and
// Linux.
- Installation.GCCInstallPath =
- (LibDir + "/" + LibSuffix + "/" + VersionText).str();
- Installation.GCCParentLibPath =
- (Installation.GCCInstallPath + "/../" + Suffix.ReversePath).str();
- Installation.SelectedMultilib = getMultilib();
-
- Installations.push_back(Installation);
+ GCCInstallPath = (LibDir + "/" + LibSuffix + "/" + VersionText).str();
+ GCCParentLibPath = (GCCInstallPath + "/../" + Suffix.ReversePath).str();
+ IsValid = true;
}
}
-
- IsValid |=
- SelectGCCInstallationDirectory(Installations, Args, SelectedInstallation);
}
bool Generic_GCC::GCCInstallationDetector::ScanGentooConfigs(
@@ -2965,12 +2916,10 @@ bool Generic_GCC::GCCInstallationDetector::ScanGentooGccConfig(
NeedsBiarchSuffix))
continue;
- SelectedInstallation.Version =
- GCCVersion::Parse(ActiveVersion.second);
- SelectedInstallation.GCCInstallPath = GentooPath;
- SelectedInstallation.GCCParentLibPath =
- GentooPath + std::string("/../../..");
- SelectedInstallation.GCCTriple.setTriple(ActiveVersion.first);
+ Version = GCCVersion::Parse(ActiveVersion.second);
+ GCCInstallPath = GentooPath;
+ GCCParentLibPath = GentooPath + std::string("/../../..");
+ GCCTriple.setTriple(ActiveVersion.first);
IsValid = true;
return true;
}
@@ -3173,9 +3122,8 @@ void Generic_GCC::AddMultilibIncludeArgs(const ArgList &DriverArgs,
// gcc TOOL_INCLUDE_DIR.
const llvm::Triple &GCCTriple = GCCInstallation.getTriple();
std::string LibPath(GCCInstallation.getParentLibPath());
- ToolChain::addSystemInclude(DriverArgs, CC1Args,
- Twine(LibPath) + "/../" + GCCTriple.str() +
- "/include");
+ addSystemInclude(DriverArgs, CC1Args,
+ Twine(LibPath) + "/../" + GCCTriple.str() + "/include");
const auto &Callback = Multilibs.includeDirsCallback();
if (Callback) {
@@ -3262,14 +3210,12 @@ Generic_GCC::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
return;
}
-static bool addLibStdCXXIncludePaths(llvm::vfs::FileSystem &vfs,
- Twine IncludeDir, StringRef Triple,
- Twine IncludeSuffix,
- const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- bool DetectDebian = false) {
-
- if (!vfs.exists(IncludeDir))
+bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
+ Twine IncludeSuffix,
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ bool DetectDebian) const {
+ if (!getVFS().exists(IncludeDir))
return false;
// Debian native gcc uses g++-multiarch-incdir.diff which uses
@@ -3281,48 +3227,39 @@ static bool addLibStdCXXIncludePaths(llvm::vfs::FileSystem &vfs,
std::string Path =
(Include + "/" + Triple + Dir.substr(Include.size()) + IncludeSuffix)
.str();
- if (DetectDebian && !vfs.exists(Path))
+ if (DetectDebian && !getVFS().exists(Path))
return false;
// GPLUSPLUS_INCLUDE_DIR
- ToolChain::addSystemInclude(DriverArgs, CC1Args, IncludeDir);
+ addSystemInclude(DriverArgs, CC1Args, IncludeDir);
// GPLUSPLUS_TOOL_INCLUDE_DIR. If Triple is not empty, add a target-dependent
// include directory.
if (DetectDebian)
- ToolChain::addSystemInclude(DriverArgs, CC1Args, Path);
+ addSystemInclude(DriverArgs, CC1Args, Path);
else if (!Triple.empty())
- ToolChain::addSystemInclude(DriverArgs, CC1Args,
- IncludeDir + "/" + Triple + IncludeSuffix);
+ addSystemInclude(DriverArgs, CC1Args,
+ IncludeDir + "/" + Triple + IncludeSuffix);
// GPLUSPLUS_BACKWARD_INCLUDE_DIR
- ToolChain::addSystemInclude(DriverArgs, CC1Args, IncludeDir + "/backward");
+ addSystemInclude(DriverArgs, CC1Args, IncludeDir + "/backward");
return true;
}
-bool Generic_GCC::addLibStdCXXIncludePaths(Twine IncludeDir, StringRef Triple,
- Twine IncludeSuffix,
- const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- bool DetectDebian) const {
- return ::addLibStdCXXIncludePaths(getVFS(), IncludeDir, Triple, IncludeSuffix,
- DriverArgs, CC1Args, DetectDebian);
-}
-
-bool Generic_GCC::GCCInstallCandidate::addGCCLibStdCxxIncludePaths(
- llvm::vfs::FileSystem &vfs, const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args, StringRef DebianMultiarch) const {
+bool Generic_GCC::addGCCLibStdCxxIncludePaths(
+ const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
+ StringRef DebianMultiarch) const {
+ assert(GCCInstallation.isValid());
// By default, look for the C++ headers in an include directory adjacent to
// the lib directory of the GCC installation. Note that this is expect to be
// equivalent to '/usr/include/c++/X.Y' in almost all cases.
- StringRef LibDir = getParentLibPath();
- StringRef InstallDir = getInstallPath();
- StringRef TripleStr = getTriple().str();
- const Multilib &Multilib = getMultilib();
- const GCCVersion &Version = getVersion();
+ StringRef LibDir = GCCInstallation.getParentLibPath();
+ StringRef InstallDir = GCCInstallation.getInstallPath();
+ StringRef TripleStr = GCCInstallation.getTriple().str();
+ const Multilib &Multilib = GCCInstallation.getMultilib();
+ const GCCVersion &Version = GCCInstallation.getVersion();
// Try /../$triple/include/c++/$version (gcc --print-multiarch is not empty).
- if (::addLibStdCXXIncludePaths(
- vfs,
+ if (addLibStdCXXIncludePaths(
LibDir.str() + "/../" + TripleStr + "/include/c++/" + Version.Text,
TripleStr, Multilib.includeSuffix(), DriverArgs, CC1Args))
return true;
@@ -3330,24 +3267,22 @@ bool Generic_GCC::GCCInstallCandidate::addGCCLibStdCxxIncludePaths(
// Try /gcc/$triple/$version/include/c++/ (gcc --print-multiarch is not
// empty). Like above but for GCC built with
// --enable-version-specific-runtime-libs.
- if (::addLibStdCXXIncludePaths(vfs,
- LibDir.str() + "/gcc/" + TripleStr + "/" +
- Version.Text + "/include/c++/",
- TripleStr, Multilib.includeSuffix(),
- DriverArgs, CC1Args))
+ if (addLibStdCXXIncludePaths(LibDir.str() + "/gcc/" + TripleStr + "/" +
+ Version.Text + "/include/c++/",
+ TripleStr, Multilib.includeSuffix(), DriverArgs,
+ CC1Args))
return true;
// Detect Debian g++-multiarch-incdir.diff.
- if (::addLibStdCXXIncludePaths(
- vfs, LibDir.str() + "/../include/c++/" + Version.Text,
- DebianMultiarch, Multilib.includeSuffix(), DriverArgs, CC1Args,
- /*Debian=*/true))
+ if (addLibStdCXXIncludePaths(LibDir.str() + "/../include/c++/" + Version.Text,
+ DebianMultiarch, Multilib.includeSuffix(),
+ DriverArgs, CC1Args, /*Debian=*/true))
return true;
// Try /../include/c++/$version (gcc --print-multiarch is empty).
- if (::addLibStdCXXIncludePaths(
- vfs, LibDir.str() + "/../include/c++/" + Version.Text, TripleStr,
- Multilib.includeSuffix(), DriverArgs, CC1Args))
+ if (addLibStdCXXIncludePaths(LibDir.str() + "/../include/c++/" + Version.Text,
+ TripleStr, Multilib.includeSuffix(), DriverArgs,
+ CC1Args))
return true;
// Otherwise, fall back on a bunch of options which don't use multiarch
@@ -3362,50 +3297,20 @@ bool Generic_GCC::GCCInstallCandidate::addGCCLibStdCxxIncludePaths(
};
for (const auto &IncludePath : LibStdCXXIncludePathCandidates) {
- if (::addLibStdCXXIncludePaths(vfs, IncludePath, TripleStr,
- Multilib.includeSuffix(), DriverArgs,
- CC1Args))
+ if (addLibStdCXXIncludePaths(IncludePath, TripleStr,
+ Multilib.includeSuffix(), DriverArgs, CC1Args))
return true;
}
return false;
}
-bool Generic_GCC::GCCInstallationDetector::
- GCCInstallationHasLibStdcxxIncludePaths(
- const GCCInstallCandidate &GCCInstallation,
- const llvm::opt::ArgList &DriverArgs) const {
- StringRef DebianMultiarch =
- TripleToDebianMultiarch(GCCInstallation.getTriple());
-
- // The following function checks for libstdc++ include paths and
- // adds them to the provided argument list. Here we just need the
- // check.
- llvm::opt::ArgStringList dummyCC1Args;
- return GCCInstallation.addGCCLibStdCxxIncludePaths(
- D.getVFS(), DriverArgs, dummyCC1Args, DebianMultiarch);
-}
-
-bool Generic_GCC::addGCCLibStdCxxIncludePaths(
- const llvm::opt::ArgList &DriverArgs,
- ll...
[truncated]
|
Contributor
Author
|
I need some time to look into the build bot failures. The new test probably needs to be restricted to a specific target. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
frederik-h
restored the
revert-145056-GCCInstallationDetector-libstdcxx
branch
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.
Reverts #145056