Replace let/nil-check patterns with `guard let` #157

dbworku · 2015-12-04T08:06:55Z

The let/nil-check pattern is more expensive and less elegant than
guard let.

The elegance goes without reasoning.
I confirmed, after some experiments with the Swift complier, that it guard let is also less expensive.

The change on LazyMapGenerator should increase performance negligibly of map().

The let/nil-check pattern is more expensive and less elegant than `guard let`.

gribozavr · 2015-12-04T08:32:18Z

stdlib/public/core/ArrayBuffer.swift

-          subscriptBaseAddress: UnsafeMutablePointer(cocoaStorageBaseAddress),
-          indices: subRange,
-          hasNativeBuffer: false)
+      guard let cocoaStorageBaseAddress = cocoa.contiguousStorage(self.indices) else {


I prefer this the old way. guard is supposed to be a quick-exit path, and in this case, if plays that role well.

gribozavr · 2015-12-04T08:35:12Z

Please update the pull request based on the comments.

Did you run the tests? build-script -RT

dbworku · 2015-12-04T13:43:10Z

@gribozavr @radex Thanks for the comments!

I will replace branches with non-trivial amount of code on either side with the if let pattern.

dbworku · 2015-12-04T16:50:42Z

@gribozavr I couldn't test this PR because build-script -RT went down went linking the build files.

[475/1921] Linking CXX shared module lib/LLVMHello.dylib
FAILED: : && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++  -fno-stack-protector -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -std=c++11 -fcolor-diagnostics -O3 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk -mmacosx-version-min=10.9 -bundle -Wl,-headerpad_max_install_names -Wl,-flat_namespace -Wl,-undefined -Wl,suppress   -Wl,-dead_strip -Wl,-exported_symbols_list,/Users/Daniel/Documents/Apple Projects/build/Ninja-ReleaseAssert/llvm-macosx-x86_64/lib/Transforms/Hello/LLVMHello.exports -o lib/LLVMHello.dylib lib/Transforms/Hello/CMakeFiles/LLVMHello.dir/Hello.cpp.o  -Wl,-rpath,@executable_path/../lib && :
clang: error: no such file or directory: 'Projects/build/Ninja-ReleaseAssert/llvm-macosx-x86_64/lib/Transforms/Hello/LLVMHello.exports'
[475/1921] Building CXX object lib/IR/CMakeFiles/LLVMCore.dir/Function.cpp.o
ninja: build stopped: subcommand failed.

gribozavr · 2015-12-04T19:11:46Z

@dbworku Are you running the latest Xcode beta?

I have never seen this issue, but it is a possibility that there is a missing dependency in LLVM's CMake scripts. Could you run build-script -RT again? If that does not help, could you delete the directory build/Ninja-ReleaseAssert/llvm-macosx-x86_64 and try again?

nadavrot · 2015-12-04T19:37:50Z

@gribozavr @dbworku Please do not commit changes to Array before running the performance test suite. Array is special and regressing the performance of array to improve readability is not okay.

The cmark_node struct and the node.h header are private.

aschwaighofer · 2015-12-04T23:25:10Z

@gribozavr @nadavrot @dbworku These changes don't look like they should affect performance. I am fine with this going in after @gribozavr gives his okay.

gribozavr · 2015-12-04T23:28:48Z

@aschwaighofer Thank you for reviewing!

I'm OK with the parts that I marked as LGTM in other comments.

Fix whitespace in test/type

Don't rely on cmark internals

I filed this under Radar rather than bugs.swift.org because it's /only/ our CI system that's failing, and external contributors have no insight into that anyway. Hopefully I'll be able to get to the bottom of this soon.

This is needed for Xcode support, even though it appeared dead within the Swift repo itself.

The let/nil-check pattern is more expensive and less elegant than `guard let`.

dbworku · 2015-12-05T15:22:59Z

@gribozavr Deleting/Reinstalling both Xcode's (release and beta) fixed my build issues.

I mangled this branch's history when rebasing after the PR #193 changes. Closing this PR in favor of a single commit that includes all of the requested changes.

Add Arrow Expressions

Refactor SyntaxRewriter

Disable an SCK project: Pods.

Replace let/nil-check patterns with guard let

8df71f8

The let/nil-check pattern is more expensive and less elegant than `guard let`.

gribozavr reviewed Dec 4, 2015
View reviewed changes

Use if let for non-trivial branches

4b4c4dc

Don't rely on cmark internals

e02ca76

The cmark_node struct and the node.h header are private.

cwillmor and others added 7 commits December 4, 2015 18:19

Merge pull request swiftlang#224 from wxxsw/whitespaces

98230e6

Fix whitespace in test/type

Merge pull request swiftlang#209 from nwellnhof/cmark

2451d06

Don't rely on cmark internals

[test] Add a Radar to the tests I disabled in f869e9e.

40584f8

I filed this under Radar rather than bugs.swift.org because it's /only/ our CI system that's failing, and external contributors have no insight into that anyway. Hopefully I'll be able to get to the bottom of this soon.

Runtime: Reinstate _stdlib_demangleName hook.

76eb96e

This is needed for Xcode support, even though it appeared dead within the Swift repo itself.

Replace let/nil-check patterns with guard let

ffc60a2

The let/nil-check pattern is more expensive and less elegant than `guard let`.

Use if let for non-trivial branches

ecc94f2

Merge remote-tracking branch 'origin/Guard-Let' into Guard-Let

65b69ac

dbworku closed this Dec 5, 2015

dbworku deleted the Guard-Let branch December 5, 2015 15:27

dabelknap pushed a commit to dabelknap/swift that referenced this pull request Nov 30, 2018

Merge pull request swiftlang#157 from dabelknap/assorted-fixes

6b5238c

Add Arrow Expressions

maldahleh pushed a commit to maldahleh/swift that referenced this pull request Oct 26, 2020

Merge pull request swiftlang#157 from ahoppen/rewriter-refactoring

4c66cf6

Refactor SyntaxRewriter

freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022

Merge pull request swiftlang#157 from atrick/disable-pods

bbdb5b4

Disable an SCK project: Pods.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace let/nil-check patterns with `guard let` #157

Replace let/nil-check patterns with `guard let` #157

dbworku commented Dec 4, 2015

gribozavr Dec 4, 2015

gribozavr commented Dec 4, 2015

dbworku commented Dec 4, 2015

dbworku commented Dec 4, 2015

gribozavr commented Dec 4, 2015

nadavrot commented Dec 4, 2015

aschwaighofer commented Dec 4, 2015

gribozavr commented Dec 4, 2015

dbworku commented Dec 5, 2015

Replace let/nil-check patterns with guard let #157

Replace let/nil-check patterns with guard let #157

Conversation

dbworku commented Dec 4, 2015

gribozavr Dec 4, 2015

Choose a reason for hiding this comment

gribozavr commented Dec 4, 2015

dbworku commented Dec 4, 2015

dbworku commented Dec 4, 2015

gribozavr commented Dec 4, 2015

nadavrot commented Dec 4, 2015

aschwaighofer commented Dec 4, 2015

gribozavr commented Dec 4, 2015

dbworku commented Dec 5, 2015

Replace let/nil-check patterns with `guard let` #157

Replace let/nil-check patterns with `guard let` #157