[ClangImporter] Import constant values for 'const' globals #80749

kubamracek · 2025-04-10T23:07:57Z

Macro-defined constants in C (#define FOO 42) get the concrete value imported (as a synthesized getter) into Swift. For global variables with initializer values (in bridging headers and in .h files in modules), we today only import them as external declarations. This PR starts importing the values of constant integer and floating-point global variables. Motivation is two-fold:

(1) Ongoing work on @const and constant-value evaluation (see [Compile Time Values] Add a new experimental feature and the parsing of the @const attribute #79753, [Compile Time Values] Implement a mandatory SIL pass to verify '@const' values #79290 and https://forums.swift.org/t/pitch-3-swift-compile-time-values/77434), where it would be very useful to be able to use imported constants from C in Swift constant expressions.
(2) Enabling more SIL-level optimizations, specifically constant folding values that depend on imported constants from C.

TODOs in this PR:

TODOs leaving as follow-up:

Start importing constexpr as members (i.e. if a constexpr global is inside a namespace or a class)

beccadax

The implementation looks fine, but I'd like to see more test coverage.

beccadax · 2025-04-10T23:26:54Z

lib/ClangImporter/ImportDecl.cpp

+        // For now, only import integer and float constants. If in the future
+        // SwiftDeclSynthesizer::createConstant becomes able to import more
+        // types, we can lift this restriction.
+        if (val && (val->isFloat() || val->isInt())) {


Do we have a test case where val would be null here? Do we have one where the type tests fail?

Also, the docs for evaluateValue() mention that it can sometimes emit notes. Do those notes reach the user if ClangImporter causes them to be emitted? If so, how do they come out?

Added the tests. My reading of evaluateValue() is that the notes are discarded (unlike e.g. checkForConstantInitialization() which Clang seems to use to actually surface the notes to the user). That seems like a good conservative default here (i.e. if the value doesn't constant fold, we silently fall back to importing as an external declaration)?

beccadax · 2025-04-10T23:27:15Z

lib/ClangImporter/ImportDecl.cpp

+              decl->getType(), ImportTypeKind::Value,
+              ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
+              isInSystemModule(dc), Bridgeability::None, ImportTypeAttrs());
+          result = synthesizer.createConstant(


Could you add a test case where an __attribute__((swift_name)) is used to import one of these constants as a member of, say, a struct? I don't think createConstant() is currently being used for anything that can have import-as-member applied to it, and although I don't see any reason why it wouldn't work, I'd like to be certain.

beccadax · 2025-04-10T23:34:12Z

test/ClangImporter/const_values.swift

+static const bool static_const_bool = true;
+static const char static_const_char = 42;
+static const long static_const_long = 42;
+static const float static_const_float = 42.0;
+static const double static_const_double = 42.0;


Some more edge cases it might be good to test:

Literal uses, say, ull to specify a different type from the constant. (Trying to make sure we don't infer a type from the literal like we might in a macro.)

Literal is too big for the type of the constant it's being assigned to (e.g. a char initialized to 500).

Initializer refers to another constant which clang could potentially evaluate to find the value.

Initializer refers to another constant which clang cannot evaluate.

Maybe not that interesting, but I think it might be nice to test some C++-y things as well:

Constexpr globals (that potentially are computed by constexpr functions)

Constants in a namespace (as namespaces are imported as enums)

Constants in class scope (static and non-static), even if we currently do not import them

👍 will add all these tests, I started gathering the list of TODOs in the PR description

I think this should now all be covered by tests.

kubamracek · 2025-04-10T23:47:06Z

@beccadax thanks for the awesome quick review, will add the coverage and find out the answers! :)

Xazax-hun

Would importing enum constants already work as expected or does that need additional work to support constant evaluation in Swift?

Xazax-hun · 2025-04-11T09:22:44Z

lib/ClangImporter/ImportDecl.cpp

+              decl->getType(), ImportTypeKind::Value,
+              ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
+              isInSystemModule(dc), Bridgeability::None, ImportTypeAttrs());
+          result = synthesizer.createConstant(


I see that we bail for static constexpr variables above. I wonder if importing them as a constant would work for the supported types.

Not for static constexpr, but for constexpr that are imported as static, i.e. they are not top-level (in a namespace, or in a class). I quickly tried just removing this restriction, and some basic stuff imports fine, so I don't see a reason why we need this restriction.

That said, I'd like to leave this as a follow-up and a separate change, so that I can explore the consequences here a bit more. Maybe @beccadax has some background on why this restriction exists in the first place?

Actually, this provides the reasoning: #60154
Though it's light on the details...

Xazax-hun · 2025-04-11T09:26:06Z

test/ClangImporter/const_values.swift

+static const bool static_const_bool = true;
+static const char static_const_char = 42;
+static const long static_const_long = 42;
+static const float static_const_float = 42.0;
+static const double static_const_double = 42.0;


Maybe not that interesting, but I think it might be nice to test some C++-y things as well:

Constexpr globals (that potentially are computed by constexpr functions)

Constants in a namespace (as namespaces are imported as enums)

Constants in class scope (static and non-static), even if we currently do not import them

kubamracek · 2025-04-11T17:18:24Z

Would importing enum constants already work as expected or does that need additional work to support constant evaluation in Swift?

Importing enum constants already works, to the best of my knowledge. See e.g. SwiftDeclConverter::VisitEnumConstantDecl. I'm trying to achieve a similar level of importing for global/static constants here.

kubamracek · 2025-04-11T17:59:26Z

@swift-ci please test

…swift + ClangImporter/const_values_cxx.swift

…adjust C++ lit tests

…s when importing const values

kubamracek requested review from jckarter, beccadax, DougGregor, artemcm and tbkka April 10, 2025 23:07

kubamracek requested review from zoecarver, egorzhdan, ian-twilightcoder, Xazax-hun, j-hui, fahadnayyar and susmonteiro as code owners April 10, 2025 23:07

beccadax requested changes Apr 10, 2025

View reviewed changes

Xazax-hun reviewed Apr 11, 2025

View reviewed changes

Xazax-hun approved these changes Apr 11, 2025

View reviewed changes

kubamracek requested a review from beccadax April 14, 2025 14:41

kubamracek added 5 commits April 14, 2025 10:01

[ClangImporter] Import constant values for 'const' globals

a39f60b

[ClangImporter] Add more test coverage in ClangImporter/const_values.…

ec24b74

…swift + ClangImporter/const_values_cxx.swift

[ClangImporter] Fix importing values for constant globals of enum types

936e911

[ClangImporter] Add more const values importing tests cases

dc65773

[ClangImporter] Skip value importing on partial/dependent C++ decls, …

8e6071e

…adjust C++ lit tests

kubamracek force-pushed the clangimport-const-values branch from 3dbc498 to 8e6071e Compare April 14, 2025 20:21

kubamracek requested review from eeckstein and hnrklssn as code owners April 14, 2025 20:21

[ClangImporter] Handle swift_wrapper annotated typedefs, skip CGFloat…

26cc36b

…s when importing const values

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ClangImporter] Import constant values for 'const' globals #80749

[ClangImporter] Import constant values for 'const' globals #80749

kubamracek commented Apr 10, 2025 •

edited

Loading

beccadax left a comment

beccadax Apr 10, 2025 •

edited

Loading

kubamracek Apr 11, 2025

beccadax Apr 10, 2025

kubamracek Apr 11, 2025

beccadax Apr 10, 2025

Xazax-hun Apr 11, 2025

kubamracek Apr 11, 2025

kubamracek Apr 11, 2025

kubamracek commented Apr 10, 2025

Xazax-hun left a comment

Xazax-hun Apr 11, 2025

kubamracek Apr 11, 2025

kubamracek Apr 11, 2025

Xazax-hun Apr 11, 2025

kubamracek commented Apr 11, 2025

kubamracek commented Apr 11, 2025

[ClangImporter] Import constant values for 'const' globals #80749

Are you sure you want to change the base?

[ClangImporter] Import constant values for 'const' globals #80749

Conversation

kubamracek commented Apr 10, 2025 • edited Loading

beccadax left a comment

Choose a reason for hiding this comment

beccadax Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubamracek commented Apr 10, 2025

Xazax-hun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubamracek commented Apr 11, 2025

kubamracek commented Apr 11, 2025

kubamracek commented Apr 10, 2025 •

edited

Loading

beccadax Apr 10, 2025 •

edited

Loading