Diagnose and forbid invalid Swift names on inits #79206
Merged
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
beccadax
requested review from
zoecarver,
hyp,
egorzhdan,
ian-twilightcoder and
Xazax-hun
as code owners
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
Xazax-hun
approved these changes
Feb 7, 2025
beccadax
requested review from
ahoppen,
bnbarham,
CodaFi,
DougGregor,
hamishknight,
rintaro and
artemcm
as code owners
|
@swift-ci please test windows platform |
|
@swift-ci please smoke test |
Initializers should always have Swift names that have the special `DeclBaseName::createConstructor()` base name. Although there is an assertion to this effect in the constructor for ConstructorDecl, ClangImporter did not actually reject custom Swift names for initializers that violated this rule. This meant that asserts compilers would crash if they encountered code with an invalid `swift_name` attribute, while release compilers would silently accept them (while excluding these decls from certain checks since lookups that were supposed to find all initializers didn’t find them). Modify ClangImporter to diagnose this condition and ignore the custom Swift name.
This commit makes a number of adjustments to how the diagnostic verifier handles source buffers and source locations. Specifically: • Files named by `-verify-additional-file` are read as late as possible so that if some other component of the compiler has already loaded the file, even in some exotic way (e.g. ClangImporter’s source buffer mirroring), it will use the same buffer. • Expectation source locations now ignore virtual files and other trickery; they are based on the source buffer and physical location in the file. Hopefully this will make `-verify-additional-file` work better on Windows. As an unintended side effect, it also changes how expectations work in tests that use `#sourceLocation()`.
beccadax
force-pushed
the
this-name-is-not-constructive
branch
from
46737f8 to
0466d5c
Compare
|
@swift-ci please test windows platform |
|
Failure seems unrelated; saw it in other Windows PR tests. @swift-ci please smoke test windows platform |
|
@swift-ci please smoke test |
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.
Initializers should always have Swift names that have the special
DeclBaseName::createConstructor()base name. Although there is an assertion to this effect in the constructor for ConstructorDecl, ClangImporter did not actually reject custom Swift names for initializers that violated this rule. This meant that asserts compilers would crash if they encountered code with an invalidswift_nameattribute, while release compilers would silently accept them (while excluding these decls from certain checks since lookups that were supposed to find all initializers didn’t find them).Modify ClangImporter to diagnose this condition and ignore the custom Swift name.