Support raw identifiers (backtick-delimited identifiers containing non-identifier characters). #76636

allevato · 2024-09-22T13:31:57Z

Implementation of SE-0451.

rintaro · 2024-12-05T20:03:29Z

include/swift/AST/Identifier.h

+  /// Returns true if this identifier contains non-identifier characters and
+  /// must always be escaped with backticks, even in contexts were other
+  /// escaped identifiers could omit backticks (like keywords as argument
+  /// labels).
+  bool mustAlwaysBeEscaped() const;


Could you add some test cases for code completion checking mustAlwaysBeEscaped() items are actually escaped in code completion suggestions?

Thanks! Those did need to be fixed.

allevato · 2024-12-24T14:09:49Z

This PR is ready except for the issue mentioned in the description of specialized C++ templates getting escaped by backticks (and now since the backticks are added to the mangling, it changes their mangled names as well). After the holidays I'll take another look at how to handle these.

allevato · 2025-01-07T17:11:21Z

The issues with imported C++ template specializations have been fixed.

allevato · 2025-01-07T17:12:24Z

swiftlang/swift-syntax#2857

@swift-ci please test

allevato · 2025-01-07T22:58:59Z

@swift-ci please test

allevato · 2025-01-08T12:54:19Z

swiftlang/swift-syntax#2857

@swift-ci please test

grynspan · 2025-01-08T13:31:21Z

Is there a named feature I can test for with hasFeature()? We may not be able to adopt until 6.3 without it because we risk breaking on older 6.2 toolchains that don't support raw identifiers.

allevato · 2025-01-08T17:49:55Z

Is there a named feature I can test for with hasFeature()? We may not be able to adopt until 6.3 without it because we risk breaking on older 6.2 toolchains that don't support raw identifiers.

Looks like LANGUAGE_FEATURE(RawIdentifiers, ...) works for this, without being an upcoming or experimental feature (it's neither). Thanks for the suggestion!

allevato · 2025-01-08T17:50:10Z

swiftlang/swift-syntax#2857

@swift-ci please test

allevato · 2025-01-08T22:21:34Z

swiftlang/swift-syntax#2857

@swift-ci please test

allevato · 2025-01-09T14:02:34Z

swiftlang/swift-syntax#2857

@swift-ci please test

allevato · 2025-01-09T17:57:35Z

swiftlang/swift-syntax#2857

@swift-ci please test macOS platform

allevato · 2025-01-10T13:54:05Z

swiftlang/swift-syntax#2857

@swift-ci please test macOS platform

allevato · 2025-01-13T14:11:23Z

@DougGregor @jckarter Can you review please, or forward along to someone who can? Thanks!

rintaro

Parser and Index related changes looks good to me.
Deferring to others for other areas.

rintaro · 2025-02-21T19:07:51Z

lib/IDE/CompletionLookup.cpp

+  if (Name.mustAlwaysBeEscaped()) {
+    // Names that are raw identifiers must always be escaped regardless of
+    // their position.
+    shouldEscapeKeywords = true;


This calls Lexer:: identifierMustAlwaysBeEscaped(StringRef) twice, in Name.mustAlwaysBeEscaped() and Builder.escapeKeyword() which is a waste. Can we avoid that? I'm fine with just manually wrapping it with back-ticks here, or you could factor out the escaping logic from Builder.escapeKeyword.

Done. I've factored out a escapeWithBackticks method in the builder so it can be called directly.

rintaro · 2025-02-21T19:08:17Z

lib/IDE/CompletionLookup.cpp

+                                     Identifier Name) {
+  if (Name.mustAlwaysBeEscaped()) {
+    SmallString<16> buffer;
+    Builder.addBaseName(Builder.escapeKeyword(Name.str(), true, buffer));


rintaro · 2025-02-21T19:12:14Z

test/SourceKit/CodeComplete/complete_raw_identifiers.swift

Could you use swift-ide-test in test/IDE for testing these? Our primary place for testing code completion results is test/IDE/complete_*.swift.
test/SourceKit/CodeComplete is for SourceKit specific functionalities, like response format, ordering or such.

Moved the tests to test/IDE and updated them to use %batch-code-completion.

rintaro

Parse/IDE/Index changes lgtm

rintaro · 2025-03-04T00:32:42Z

lib/IDE/CodeCompletionResultBuilder.h

@@ -382,18 +391,16 @@ class CodeCompletionResultBuilder {
 #define KEYWORD(kw) .Case(#kw, true)
      shouldEscape = llvm::StringSwitch<bool>(Word)
 #include "swift/AST/TokenKinds.def"
-        .Default(false);
+                         .Default(Lexer::identifierMustAlwaysBeEscaped(Word));


Nit: indentation

This is what clang-format wants, apparently, because it's treated as a continuation of the expression llvm::StringSwitch... above.

DougGregor · 2025-03-04T21:03:41Z

@allevato I'm so sorry, I thought this review had been picked up before.

DougGregor

This is looking great, please go ahead and merge

DougGregor · 2025-03-11T18:09:49Z

lib/AST/ASTMangler.cpp

@@ -3062,7 +3062,8 @@ void ASTMangler::appendAnyGenericType(const GenericTypeDecl *decl,
      // `ClassTemplateSpecializationDecl`'s name does not include information about
      // template arguments, and in order to prevent name clashes we use the
      // name of the Swift decl which does include template arguments.
-      appendIdentifier(nominal->getName().str());
+      appendIdentifier(nominal->getName().str(),
+                       /*allowRawIdentifiers=*/false);


I feel like we should revisit the naming of imported class template specializations in light of raw identifiers, but I don't want to bring any of that discussion into this pull request.

lib/Index/Index.cpp

DougGregor · 2025-03-11T18:23:08Z

lib/Sema/TypeCheckDeclObjC.cpp

-    if (auto attr = proto->getAttrs().getAttribute<ObjCAttr>()) {
+    auto attr = proto->getAttrs().getAttribute<ObjCAttr>();
+    if (attr) {
+      checkObjCNameValidity(proto, attr);


This checkObjCNameValidity should be outside of the if so that we diagnose, e.g.,

@objc protocol `some protocol` { }

right?

[EDIT: No, I'm wrong, because you have a test like this. I'm not sure how it works, though]

This is just checking for the presence of any @objc attr, which may or may not have an explicit name (the latter is checked by checkObjCNameValidity), so I think this is working as intended. Unless I'm misunderstanding your comment?

DougGregor · 2025-03-11T18:24:52Z

test/Parse/escaped_identifiers.swift

+func `+`(lhs: Int, rhs: Int) -> Int  // expected-error{{expected identifier in function declaration}}
+
+@propertyWrapper
+struct `@PoorlyNamedWrapper`<`The Value`> {


It's hideous! But makes a nice test case, thanks

allevato · 2025-03-11T20:01:53Z

@swift-ci please test

Raw identifiers are backtick-delimited identifiers that can contain any non-identifier character other than the backtick itself, CR, LF, or other non-printable ASCII code units, and which are also not composed entirely of operator characters.

This requires no additional work beyond just parsing the identifier because Clang module maps already support such names in quotes.

The original module names themselves must still be valid unescaped identifiers; most of the serialization logic in the compiler depends on the name of a module matching its name on the file system, and it would be very complex to turn escaped identifiers into file-safe names.

… JSON file. For build systems that already generate these files, it makes sense to include the aliases so that the map file serves as a comprehensive index of how the module inputs are referenced.

`#fileID` never accounted for the possibility that someone one have a module alias _itself_, so it always generated the module's real (physical) name. This _technically_ changes the behavior of `#fileID` for self-aliased modules, but since nobody would have ever had a reason to do that before raw identifiers, it's unlikely that this change would affect anyone in practice.

If a decl is exported to Objective-C (explicitly or implicitly), it must be given an explicit name that is a valid Objective-C identifier.

Imported C++ template specializations receive identifiers that contain their type signature; e.g., `X<Y, Z>`. Since this means the identifier contains non-identifier characters, the new behavior was trying to escape them with backticks in ASTPrinter, ASTMangler, and the runtime metadata. This pulls that back to preserve the current behavior for specifically those types.

This lets clients test `#if hasFeature(RawIdentifiers)` to determine compiler support.

allevato · 2025-03-11T22:17:28Z

@swift-ci please test

allevato · 2025-03-12T11:32:03Z

@swift-ci please test macOS platform

This is the companion to the frontend changes for SE-0451 in swiftlang/swift#76636. The first component of a `-module-alias` flag is allowed to be a raw identifier (without the backticks), which allows for uses like the following: ``` import `//some/module:name` ``` The *target* of the alias (the actual module name) must still be a valid Swift identifier, since that corresponds to file system artifacts that need stricter naming conventions.

rintaro reviewed Dec 5, 2024

View reviewed changes

allevato force-pushed the rich-identifiers branch from 163c6f9 to cd7ed4e Compare December 24, 2024 14:06

allevato marked this pull request as ready for review December 24, 2024 14:08

allevato requested review from ahoppen, bnbarham, hamishknight, jckarter, CodaFi, DougGregor, artemcm, tshortli, rjmccall, xymus, hborla, slavapestov and xedin as code owners December 24, 2024 14:08

allevato force-pushed the rich-identifiers branch from d392874 to aac1fdb Compare January 7, 2025 17:10

allevato force-pushed the rich-identifiers branch from f5ee709 to 7e6f8ba Compare January 8, 2025 22:21

allevato force-pushed the rich-identifiers branch from d31b241 to 82eb01b Compare January 12, 2025 15:10

rintaro reviewed Feb 21, 2025

View reviewed changes

allevato force-pushed the rich-identifiers branch from 82eb01b to c25931e Compare February 24, 2025 01:10

rintaro reviewed Mar 4, 2025

View reviewed changes

allevato force-pushed the rich-identifiers branch from c25931e to 0c524e5 Compare March 4, 2025 22:05

DougGregor approved these changes Mar 11, 2025

View reviewed changes

allevato force-pushed the rich-identifiers branch from 0c524e5 to e701d40 Compare March 11, 2025 19:55

allevato enabled auto-merge March 11, 2025 20:01

allevato disabled auto-merge March 11, 2025 21:17

allevato and others added 9 commits March 11, 2025 17:18

Add support for raw identifiers.

d94bd80

Raw identifiers are backtick-delimited identifiers that can contain any non-identifier character other than the backtick itself, CR, LF, or other non-printable ASCII code units, and which are also not composed entirely of operator characters.

Add a test importing a Clang module with non-identifier characters.

3292615

This requires no additional work beyond just parsing the identifier because Clang module maps already support such names in quotes.

Allow module aliases to be expressed in the explicit Swift module map…

8752920

… JSON file. For build systems that already generate these files, it makes sense to include the aliases so that the map file serves as a comprehensive index of how the module inputs are referenced.

[Sema] Validate names of @objc declarations with raw identifiers.

a68347f

If a decl is exported to Objective-C (explicitly or implicitly), it must be given an explicit name that is a valid Objective-C identifier.

Add RawIdentifiers as a language feature.

7797664

This lets clients test `#if hasFeature(RawIdentifiers)` to determine compiler support.

Final tweaks from review comments.

d71b42e

allevato force-pushed the rich-identifiers branch from e701d40 to d71b42e Compare March 11, 2025 21:53

allevato enabled auto-merge March 11, 2025 22:17

allevato mentioned this pull request Mar 12, 2025

Allow module aliases to be raw identifiers. swiftlang/swift-driver#1836

Merged

allevato merged commit 68876a6 into swiftlang:main Mar 12, 2025
4 of 5 checks passed

allevato deleted the rich-identifiers branch March 12, 2025 19:03

stevapple mentioned this pull request May 3, 2025

Raw identifier support (from Swift 6.2) mattgallagher/CwlDemangle#23

Open

Support raw identifiers (backtick-delimited identifiers containing non-identifier characters). #76636

Support raw identifiers (backtick-delimited identifiers containing non-identifier characters). #76636

Uh oh!

Conversation

allevato commented Sep 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rintaro Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allevato commented Dec 24, 2024

Uh oh!

allevato commented Jan 7, 2025

Uh oh!

allevato commented Jan 7, 2025

Uh oh!

allevato commented Jan 7, 2025

Uh oh!

allevato commented Jan 8, 2025

Uh oh!

grynspan commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allevato commented Jan 8, 2025

Uh oh!

allevato commented Jan 8, 2025

Uh oh!

allevato commented Jan 8, 2025

Uh oh!

allevato commented Jan 9, 2025

Uh oh!

allevato commented Jan 9, 2025

Uh oh!

allevato commented Jan 10, 2025

Uh oh!

allevato commented Jan 13, 2025

Uh oh!

rintaro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor commented Mar 4, 2025

Uh oh!

DougGregor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allevato commented Mar 11, 2025

Uh oh!

allevato commented Mar 11, 2025

Uh oh!

allevato commented Sep 22, 2024 •

edited

Loading

rintaro Dec 5, 2024 •

edited

Loading

grynspan commented Jan 8, 2025 •

edited

Loading