[strliteral-upgrade] Change Strings.h constants to be constexpr inste… #15975
Conversation
|
apple/swift-llvm#84 @swift-ci smoke test |
|
I looked into why this was not necessary on master-next. It was necessary! We just worked around it. See: 609b151 |
There was a problem hiding this comment.
LGTM. Personally, I'd try mechanically changing all static const strings in the headers to constexpr const strings, but reasonable people could disagree about whether this is wise or not.
Also, can you please revert the withInnerNUL workaround after landing the constexpr fix?
include/swift/Strings.h
Outdated
| @@ -16,72 +16,72 @@ | |||
| namespace swift { | |||
|
|
|||
| /// The extension for serialized modules. | |||
| static const char SERIALIZED_MODULE_EXTENSION[] = "swiftmodule"; | |||
| constexpr const char SERIALIZED_MODULE_EXTENSION[] = "swiftmodule"; | |||
There was a problem hiding this comment.
These should still be static, thanks! Otherwise they'll get emitted into every file that includes this one.
gottesmm
force-pushed
the
strliteral-upgrade
branch
from
8c9e7fc to
c2b797f
Compare
|
@swift-ci smoke test |
gottesmm
force-pushed
the
strliteral-upgrade
branch
2 times, most recently
from
20f42bd to
030e2be
Compare
…c instead of static. Otherwise they can not be passed to StringLiteral. That would prevent me from landing DaveZ's StringSwitch improvements from apple/swift-llvm#84 since StringSwitch only takes StringLiterals now.
gottesmm
force-pushed
the
strliteral-upgrade
branch
from
030e2be to
b402696
Compare
|
apple/swift-llvm#84 @swift-ci smoke test |
2 similar comments
|
apple/swift-llvm#84 @swift-ci smoke test |
|
apple/swift-llvm#84 @swift-ci smoke test |
|
@davezarzycki That is going to cause a merge conflict... so yes, I need to fix it ; ). |
|
Nope, I was wrong, but I will fix it. |
…ad of static.
Otherwise they can not be passed to StringLiteral. That would prevent me from
landing DaveZ's StringSwitch improvements from apple/swift-llvm#84 since
StringSwitch only takes StringLiterals now.