Skip to content

[libc++] Improve the verbosity of configuration errors when a compiler flag is not supported #66379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 14, 2023

If an assertion fails during the configuration of the test suite because the compiler doesn't support a flag (or the compiler configuration is borked entirely), we will now print additional context to help debug the issue instead of just saying "The compiler doesn't support the flag" and bailing.

@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 14, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 14, 2023

@llvm/pr-subscribers-libcxx

Changes If an assertion fails during the configuration of the test suite because the compiler doesn't support a flag (or the compiler configuration is borked entirely), we will now print additional context to help debug the issue instead of just saying "The compiler doesn't support the flag" and bailing. -- Full diff: https://github.com//pull/66379.diff

1 Files Affected:

  • (modified) libcxx/utils/libcxx/test/dsl.py (+21-22)
diff --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 847cebf5962f6aa..b0e7cea750a1e41 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -203,6 +203,17 @@ def programSucceeds(config, program, args=None):
 
 
 @_memoizeExpensiveOperation(lambda c, f: (c.substitutions, c.environment, f))
+def tryCompileFlag(config, flag):
+    """
+    Try using the given compiler flag and return the exit code along with stdout and stderr.
+    """
+    with _makeConfigTest(config) as test:
+        out, err, exitCode, timeoutInfo, _ = _executeWithFakeConfig(test, [
+            "%{{cxx}} -xc++ {} -Werror -fsyntax-only %{{flags}} %{{compile_flags}} {}".format(os.devnull, flag)
+        ])
+        return exitCode, out, err
+
+
 def hasCompileFlag(config, flag):
     """
     Return whether the compiler in the configuration supports a given compiler flag.
@@ -210,16 +221,8 @@ def hasCompileFlag(config, flag):
     This is done by executing the %{cxx} substitution with the given flag and
     checking whether that succeeds.
     """
-    with _makeConfigTest(config) as test:
-        out, err, exitCode, timeoutInfo, _ = _executeWithFakeConfig(
-            test,
-            [
-                "%{{cxx}} -xc++ {} -Werror -fsyntax-only %{{flags}} %{{compile_flags}} {}".format(
-                    os.devnull, flag
-                )
-            ],
-        )
-        return exitCode == 0
+    (exitCode, _, _) = tryCompileFlag(config, flag)
+    return exitCode == 0
 
 
 @_memoizeExpensiveOperation(lambda c, s: (c.substitutions, c.environment, s))
@@ -388,6 +391,10 @@ def pretty(self, config, litParams):
         pass
 
 
+def _ensureFlagIsSupported(config, flag):
+    (exitCode, out, err) = tryCompileFlag(config, flag)
+    assert exitCode == 0, f"Trying to enable compiler flag {flag}, which is not supported. stdout was:\n{out}\n\nstderr was:\n{err}"
+
 class AddFeature(ConfigAction):
     """
     This action defines the given Lit feature when running the test suite.
@@ -427,9 +434,7 @@ def __init__(self, flag):
 
     def applyTo(self, config):
         flag = self._getFlag(config)
-        assert hasCompileFlag(
-            config, flag
-        ), "Trying to enable flag {}, which is not supported".format(flag)
+        _ensureFlagIsSupported(config, flag)
         config.substitutions = _appendToSubstitution(
             config.substitutions, "%{flags}", flag
         )
@@ -473,9 +478,7 @@ def __init__(self, flag):
 
     def applyTo(self, config):
         flag = self._getFlag(config)
-        assert hasCompileFlag(
-            config, flag
-        ), "Trying to enable compile flag {}, which is not supported".format(flag)
+        _ensureFlagIsSupported(config, flag)
         config.substitutions = _appendToSubstitution(
             config.substitutions, "%{compile_flags}", flag
         )
@@ -497,9 +500,7 @@ def __init__(self, flag):
 
     def applyTo(self, config):
         flag = self._getFlag(config)
-        assert hasCompileFlag(
-            config, flag
-        ), "Trying to enable link flag {}, which is not supported".format(flag)
+        _ensureFlagIsSupported(config, flag)
         config.substitutions = _appendToSubstitution(
             config.substitutions, "%{link_flags}", flag
         )
@@ -521,9 +522,7 @@ def __init__(self, flag):
 
     def applyTo(self, config):
         flag = self._getFlag(config)
-        assert hasCompileFlag(
-            config, flag
-        ), "Trying to enable link flag {}, which is not supported".format(flag)
+        _ensureFlagIsSupported(config, flag)
         config.substitutions = _prependToSubstitution(
             config.substitutions, "%{link_flags}", flag
         )

…r flag is not supported

If an assertion fails during the configuration of the test suite because
the compiler doesn't support a flag (or the compiler configuration is
borked entirely), we will now print additional context to help debug the
issue instead of just saying "The compiler doesn't support the flag" and
bailing.
@ldionne ldionne force-pushed the review/improve-lit-configuration-errors branch from 3ba34e2 to 8bcfaa1 Compare September 14, 2023 14:17
@ldionne ldionne merged commit b64bf89 into llvm:main Sep 15, 2023
@ldionne ldionne deleted the review/improve-lit-configuration-errors branch September 15, 2023 14:15
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…r flag is not supported (llvm#66379)

If an assertion fails during the configuration of the test suite because
the compiler doesn't support a flag (or the compiler configuration is
borked entirely), we will now print additional context to help debug the
issue instead of just saying "The compiler doesn't support the flag" and
bailing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants