Skip to content

Commit a9ef40b

Browse files
82marbag82marbag
authored
Lifetimes in builders (#3249)
## Motivation and Context We're not handling lifetimes in builders of structures. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Signed-off-by: Daniele Ahmed <[email protected]>
1 parent 8754c99 commit a9ef40b

File tree

7 files changed

+85
-51
lines changed

7 files changed

+85
-51
lines changed

‎codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustReservedWords.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class RustReservedWordSymbolProvider(
6969
"RustReservedWordSymbolProvider should only run once"
7070
}
7171

72-
var renamedSymbol = internal.toSymbol(shape)
72+
val renamedSymbol = internal.toSymbol(shape)
7373
return when (shape) {
7474
is MemberShape -> {
7575
val container = model.expectShape(shape.container)

‎codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,32 +88,16 @@ open class StructureGenerator(
8888
renderStructure()
8989
}
9090

91-
/**
92-
* Search for lifetimes used by the members of the struct and generate a declaration.
93-
* e.g. `<'a, 'b>`
94-
*/
95-
private fun lifetimeDeclaration(): String {
96-
val lifetimes = members
97-
.map { symbolProvider.toSymbol(it).rustType().innerReference() }
98-
.mapNotNull {
99-
when (it) {
100-
is RustType.Reference -> it.lifetime
101-
else -> null
102-
}
103-
}.toSet().sorted()
104-
return if (lifetimes.isNotEmpty()) {
105-
"<${lifetimes.joinToString { "'$it" }}>"
106-
} else {
107-
""
108-
}
109-
}
110-
11191
/**
11292
* Render a custom debug implementation
11393
* When [SensitiveTrait] support is required, render a custom debug implementation to redact sensitive data
11494
*/
11595
private fun renderDebugImpl() {
116-
writer.rustBlock("impl ${lifetimeDeclaration()} #T for $name ${lifetimeDeclaration()}", RuntimeType.Debug) {
96+
val lifetime = shape.lifetimeDeclaration(symbolProvider)
97+
writer.rustBlock(
98+
"impl ${shape.lifetimeDeclaration(symbolProvider)} #T for $name $lifetime",
99+
RuntimeType.Debug,
100+
) {
117101
writer.rustBlock("fn fmt(&self, f: &mut #1T::Formatter<'_>) -> #1T::Result", RuntimeType.stdFmt) {
118102
rust("""let mut formatter = f.debug_struct(${name.dq()});""")
119103
members.forEach { member ->
@@ -134,8 +118,13 @@ open class StructureGenerator(
134118
if (accessorMembers.isEmpty()) {
135119
return
136120
}
137-
val lifetimes = lifetimeDeclaration()
138-
writer.rustBlock("impl $lifetimes $name $lifetimes") {
121+
writer.rustBlock(
122+
"impl ${shape.lifetimeDeclaration(symbolProvider)} $name ${
123+
shape.lifetimeDeclaration(
124+
symbolProvider,
125+
)
126+
}",
127+
) {
139128
// Render field accessor methods
140129
forEachMember(accessorMembers) { member, memberName, memberSymbol ->
141130
val memberType = memberSymbol.rustType()
@@ -146,6 +135,7 @@ open class StructureGenerator(
146135
unwrapOrDefault = true
147136
memberType.stripOuter<RustType.Option>().asDeref().asRef()
148137
}
138+
149139
memberType.isCopy() -> memberType
150140
memberType is RustType.Option && memberType.member.isDeref() -> memberType.asDeref()
151141
memberType.isDeref() -> memberType.asDeref().asRef()
@@ -188,7 +178,7 @@ open class StructureGenerator(
188178
writer.deprecatedShape(shape)
189179
containerMeta.render(writer)
190180

191-
writer.rustBlock("struct $name ${lifetimeDeclaration()}") {
181+
writer.rustBlock("struct $name ${shape.lifetimeDeclaration(symbolProvider)}") {
192182
writer.forEachMember(members) { member, memberName, memberSymbol ->
193183
renderStructureMember(writer, member, memberName, memberSymbol)
194184
}
@@ -223,3 +213,19 @@ open class StructureGenerator(
223213
)
224214
}
225215
}
216+
217+
/**
218+
* Search for lifetimes used by the members of the struct and generate a declaration.
219+
* e.g. `<'a, 'b>`
220+
*/
221+
fun StructureShape.lifetimeDeclaration(symbolProvider: RustSymbolProvider): String {
222+
val lifetimes = this.members()
223+
.mapNotNull { symbolProvider.toSymbol(it).rustType().innerReference()?.let { it as RustType.Reference } }
224+
.mapNotNull { it.lifetime }
225+
.toSet().sorted()
226+
return if (lifetimes.isNotEmpty()) {
227+
"<${lifetimes.joinToString { "'$it" }}>"
228+
} else {
229+
""
230+
}
231+
}

‎codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/transformers/OperationNormalizer.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ object OperationNormalizer {
6262
check(
6363
shapeConflict == null,
6464
) {
65-
"shape $shapeConflict conflicted with an existing shape in the model (${model.getShape(shapeConflict!!.id)}. This is a bug."
65+
"shape $shapeConflict conflicted with an existing shape in the model (${model.expectShape(shapeConflict!!.id)}). This is a bug."
6666
}
6767
val modelWithOperationInputs = model.toBuilder().addShapes(newShapes).build()
6868
return transformer.mapShapes(modelWithOperationInputs) {

‎codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import software.amazon.smithy.model.traits.LengthTrait
3232
import software.amazon.smithy.model.transform.ModelTransformer
3333
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
3434
import software.amazon.smithy.rust.codegen.core.rustlang.implBlock
35+
import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
3536
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
3637
import software.amazon.smithy.rust.codegen.core.smithy.CoreRustSettings
3738
import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker
@@ -41,6 +42,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.generators.EnumGenerator
4142
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator
4243
import software.amazon.smithy.rust.codegen.core.smithy.generators.UnionGenerator
4344
import software.amazon.smithy.rust.codegen.core.smithy.generators.error.ErrorImplGenerator
45+
import software.amazon.smithy.rust.codegen.core.smithy.generators.lifetimeDeclaration
4446
import software.amazon.smithy.rust.codegen.core.smithy.protocols.ProtocolGeneratorFactory
4547
import software.amazon.smithy.rust.codegen.core.smithy.transformers.EventStreamNormalizer
4648
import software.amazon.smithy.rust.codegen.core.smithy.transformers.OperationNormalizer
@@ -243,7 +245,10 @@ open class ServerCodegenVisitor(
243245
logger.log(logMessage.level, logMessage.message)
244246
}
245247
if (validationResult.shouldAbort) {
246-
throw CodegenException("Unsupported constraints feature used; see error messages above for resolution", validationResult)
248+
throw CodegenException(
249+
"Unsupported constraints feature used; see error messages above for resolution",
250+
validationResult,
251+
)
247252
}
248253
}
249254

@@ -328,7 +333,8 @@ open class ServerCodegenVisitor(
328333
serverBuilderGenerator.render(rustCrate, writer)
329334

330335
if (codegenContext.settings.codegenConfig.publicConstrainedTypes) {
331-
writer.implBlock(codegenContext.symbolProvider.toSymbol(shape)) {
336+
val lifetimes = shape.lifetimeDeclaration(codegenContext.symbolProvider)
337+
writer.rustBlock("impl $lifetimes ${codegenContext.symbolProvider.toSymbol(shape).name} $lifetimes") {
332338
serverBuilderGenerator.renderConvenienceMethod(this)
333339
}
334340
}
@@ -372,7 +378,11 @@ open class ServerCodegenVisitor(
372378

373379
if (renderUnconstrainedList) {
374380
logger.info("[rust-server-codegen] Generating an unconstrained type for collection shape $shape")
375-
rustCrate.withModuleOrWithStructureBuilderModule(ServerRustModule.UnconstrainedModule, shape, codegenContext) {
381+
rustCrate.withModuleOrWithStructureBuilderModule(
382+
ServerRustModule.UnconstrainedModule,
383+
shape,
384+
codegenContext,
385+
) {
376386
UnconstrainedCollectionGenerator(
377387
codegenContext,
378388
rustCrate.createInlineModuleCreator(),
@@ -382,7 +392,11 @@ open class ServerCodegenVisitor(
382392

383393
if (!isDirectlyConstrained) {
384394
logger.info("[rust-server-codegen] Generating a constrained type for collection shape $shape")
385-
rustCrate.withModuleOrWithStructureBuilderModule(ServerRustModule.ConstrainedModule, shape, codegenContext) {
395+
rustCrate.withModuleOrWithStructureBuilderModule(
396+
ServerRustModule.ConstrainedModule,
397+
shape,
398+
codegenContext,
399+
) {
386400
PubCrateConstrainedCollectionGenerator(
387401
codegenContext,
388402
rustCrate.createInlineModuleCreator(),
@@ -427,7 +441,11 @@ open class ServerCodegenVisitor(
427441

428442
if (renderUnconstrainedMap) {
429443
logger.info("[rust-server-codegen] Generating an unconstrained type for map $shape")
430-
rustCrate.withModuleOrWithStructureBuilderModule(ServerRustModule.UnconstrainedModule, shape, codegenContext) {
444+
rustCrate.withModuleOrWithStructureBuilderModule(
445+
ServerRustModule.UnconstrainedModule,
446+
shape,
447+
codegenContext,
448+
) {
431449
UnconstrainedMapGenerator(
432450
codegenContext,
433451
rustCrate.createInlineModuleCreator(),
@@ -437,7 +455,11 @@ open class ServerCodegenVisitor(
437455

438456
if (!isDirectlyConstrained) {
439457
logger.info("[rust-server-codegen] Generating a constrained type for map $shape")
440-
rustCrate.withModuleOrWithStructureBuilderModule(ServerRustModule.ConstrainedModule, shape, codegenContext) {
458+
rustCrate.withModuleOrWithStructureBuilderModule(
459+
ServerRustModule.ConstrainedModule,
460+
shape,
461+
codegenContext,
462+
) {
441463
PubCrateConstrainedMapGenerator(
442464
codegenContext,
443465
rustCrate.createInlineModuleCreator(),
@@ -575,7 +597,9 @@ open class ServerCodegenVisitor(
575597
*/
576598
open fun protocolTests() {
577599
rustCrate.withModule(ServerRustModule.Operation) {
578-
ServerProtocolTestGenerator(codegenContext, protocolGeneratorFactory.support(), protocolGenerator).render(this)
600+
ServerProtocolTestGenerator(codegenContext, protocolGeneratorFactory.support(), protocolGenerator).render(
601+
this,
602+
)
579603
}
580604
}
581605

‎codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.withBlock
3131
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
3232
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
3333
import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
34+
import software.amazon.smithy.rust.codegen.core.smithy.generators.lifetimeDeclaration
3435
import software.amazon.smithy.rust.codegen.core.smithy.isOptional
3536
import software.amazon.smithy.rust.codegen.core.smithy.isRustBoxed
3637
import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained
@@ -147,6 +148,7 @@ class ServerBuilderGenerator(
147148
private val isBuilderFallible = hasFallibleBuilder(shape, model, symbolProvider, takeInUnconstrainedTypes)
148149
private val serverBuilderConstraintViolations =
149150
ServerBuilderConstraintViolations(codegenContext, shape, takeInUnconstrainedTypes, customValidationExceptionWithReasonConversionGenerator)
151+
private val lifetime = shape.lifetimeDeclaration(symbolProvider)
150152

151153
private val codegenScope = arrayOf(
152154
"RequestRejection" to protocol.requestRejection(codegenContext.runtimeConfig),
@@ -196,11 +198,11 @@ class ServerBuilderGenerator(
196198
it == RuntimeType.Debug || it == RuntimeType.Clone
197199
} + RuntimeType.Default
198200
Attribute(derive(builderDerives)).render(writer)
199-
writer.rustBlock("${visibility.toRustQualifier()} struct Builder") {
201+
writer.rustBlock("${visibility.toRustQualifier()} struct Builder$lifetime") {
200202
members.forEach { renderBuilderMember(this, it) }
201203
}
202204

203-
writer.rustBlock("impl Builder") {
205+
writer.rustBlock("impl $lifetime Builder $lifetime") {
204206
for (member in members) {
205207
if (publicConstrainedTypes) {
206208
renderBuilderMemberFn(this, member)
@@ -262,15 +264,15 @@ class ServerBuilderGenerator(
262264
self.build_enforcing_all_constraints()
263265
}
264266
""",
265-
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol),
267+
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol, lifetime),
266268
)
267269
renderBuildEnforcingAllConstraintsFn(implBlockWriter)
268270
}
269271

270272
private fun renderBuildEnforcingAllConstraintsFn(implBlockWriter: RustWriter) {
271273
implBlockWriter.rustBlockTemplate(
272274
"fn build_enforcing_all_constraints(self) -> #{ReturnType:W}",
273-
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol),
275+
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol, lifetime),
274276
) {
275277
conditionalBlock("Ok(", ")", conditional = isBuilderFallible) {
276278
coreBuilder(this)
@@ -280,7 +282,7 @@ class ServerBuilderGenerator(
280282

281283
fun renderConvenienceMethod(implBlock: RustWriter) {
282284
implBlock.docs("Creates a new builder-style object to manufacture #D.", structureSymbol)
283-
implBlock.rustBlock("pub fn builder() -> #T", builderSymbol) {
285+
implBlock.rustBlock("pub fn builder() -> #T $lifetime", builderSymbol) {
284286
write("#T::default()", builderSymbol)
285287
}
286288
}
@@ -413,10 +415,10 @@ class ServerBuilderGenerator(
413415
private fun renderTryFromBuilderImpl(writer: RustWriter) {
414416
writer.rustTemplate(
415417
"""
416-
impl #{TryFrom}<Builder> for #{Structure} {
418+
impl #{TryFrom}<Builder $lifetime> for #{Structure}$lifetime {
417419
type Error = ConstraintViolation;
418420
419-
fn try_from(builder: Builder) -> Result<Self, Self::Error> {
421+
fn try_from(builder: Builder $lifetime) -> Result<Self, Self::Error> {
420422
builder.build()
421423
}
422424
}
@@ -428,7 +430,7 @@ class ServerBuilderGenerator(
428430
private fun renderFromBuilderImpl(writer: RustWriter) {
429431
writer.rustTemplate(
430432
"""
431-
impl #{From}<Builder> for #{Structure} {
433+
impl #{From}<Builder $lifetime> for #{Structure} $lifetime {
432434
fn from(builder: Builder) -> Self {
433435
builder.build()
434436
}

‎codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ import software.amazon.smithy.rust.codegen.server.smithy.hasPublicConstrainedWra
5757
/**
5858
* Returns a writable to render the return type of the server builders' `build()` method.
5959
*/
60-
fun buildFnReturnType(isBuilderFallible: Boolean, structureSymbol: Symbol) = writable {
60+
fun buildFnReturnType(isBuilderFallible: Boolean, structureSymbol: Symbol, lifetime: String) = writable {
6161
if (isBuilderFallible) {
62-
rust("Result<#T, ConstraintViolation>", structureSymbol)
62+
rust("Result<#T $lifetime, ConstraintViolation>", structureSymbol)
6363
} else {
64-
rust("#T", structureSymbol)
64+
rust("#T $lifetime", structureSymbol)
6565
}
6666
}
6767

‎codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorWithoutPublicConstrainedTypes.kt

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.withBlock
2525
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
2626
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
2727
import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
28+
import software.amazon.smithy.rust.codegen.core.smithy.generators.lifetimeDeclaration
2829
import software.amazon.smithy.rust.codegen.core.smithy.isOptional
2930
import software.amazon.smithy.rust.codegen.core.smithy.makeOptional
3031
import software.amazon.smithy.rust.codegen.core.smithy.module
@@ -84,6 +85,7 @@ class ServerBuilderGeneratorWithoutPublicConstrainedTypes(
8485
private val isBuilderFallible = hasFallibleBuilder(shape, symbolProvider)
8586
private val serverBuilderConstraintViolations =
8687
ServerBuilderConstraintViolations(codegenContext, shape, builderTakesInUnconstrainedTypes = false, validationExceptionConversionGenerator)
88+
private val lifetime = shape.lifetimeDeclaration(symbolProvider)
8789

8890
private val codegenScope = arrayOf(
8991
"RequestRejection" to protocol.requestRejection(codegenContext.runtimeConfig),
@@ -152,15 +154,15 @@ class ServerBuilderGeneratorWithoutPublicConstrainedTypes(
152154
self.build_enforcing_required_and_enum_traits()
153155
}
154156
""",
155-
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol),
157+
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol, lifetime),
156158
)
157159
renderBuildEnforcingRequiredAndEnumTraitsFn(implBlockWriter)
158160
}
159161

160162
private fun renderBuildEnforcingRequiredAndEnumTraitsFn(implBlockWriter: RustWriter) {
161163
implBlockWriter.rustBlockTemplate(
162164
"fn build_enforcing_required_and_enum_traits(self) -> #{ReturnType:W}",
163-
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol),
165+
"ReturnType" to buildFnReturnType(isBuilderFallible, structureSymbol, lifetime),
164166
) {
165167
conditionalBlock("Ok(", ")", conditional = isBuilderFallible) {
166168
coreBuilder(this)
@@ -199,7 +201,7 @@ class ServerBuilderGeneratorWithoutPublicConstrainedTypes(
199201

200202
fun renderConvenienceMethod(implBlock: RustWriter) {
201203
implBlock.docs("Creates a new builder-style object to manufacture #D.", structureSymbol)
202-
implBlock.rustBlock("pub fn builder() -> #T", builderSymbol) {
204+
implBlock.rustBlock("pub fn builder() -> #T $lifetime", builderSymbol) {
203205
write("#T::default()", builderSymbol)
204206
}
205207
}
@@ -237,10 +239,10 @@ class ServerBuilderGeneratorWithoutPublicConstrainedTypes(
237239
private fun renderTryFromBuilderImpl(writer: RustWriter) {
238240
writer.rustTemplate(
239241
"""
240-
impl #{TryFrom}<Builder> for #{Structure} {
242+
impl #{TryFrom}<Builder $lifetime> for #{Structure}$lifetime {
241243
type Error = ConstraintViolation;
242244
243-
fn try_from(builder: Builder) -> Result<Self, Self::Error> {
245+
fn try_from(builder: Builder $lifetime) -> Result<Self, Self::Error> {
244246
builder.build()
245247
}
246248
}
@@ -252,8 +254,8 @@ class ServerBuilderGeneratorWithoutPublicConstrainedTypes(
252254
private fun renderFromBuilderImpl(writer: RustWriter) {
253255
writer.rustTemplate(
254256
"""
255-
impl #{From}<Builder> for #{Structure} {
256-
fn from(builder: Builder) -> Self {
257+
impl #{From}<Builder $lifetime> for #{Structure}$lifetime {
258+
fn from(builder: Builder $lifetime) -> Self {
257259
builder.build()
258260
}
259261
}

0 commit comments

Comments
 (0)