Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-7562] @discardableResult is disregarded for a required init method #50104

Closed
swift-ci opened this issue Apr 28, 2018 · 13 comments
Closed

[SR-7562] @discardableResult is disregarded for a required init method #50104

swift-ci opened this issue Apr 28, 2018 · 13 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself good first issue Good for newcomers

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-7562
Radar None
Original Reporter Vandad (JIRA User)
Type Bug
Status Resolved
Resolution Done
Environment

swift --version

Apple Swift version 4.1 (swiftlang-902.0.48 clang-902.0.37.1)

Target: x86_64-apple-darwin17.5.0

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, StarterBug
Assignee @vguerra
Priority Medium

md5: bddad59f549a8118d77ef9d4bf5040fb

Issue Description:

My problem is sort of related to the following issue: SR-2198

Consider the following example:

protocol HasInitMethod {
    associatedtype Input
    init(input: Input)
}

class Foo<Input>: HasInitMethod {
    @discardableResult required init(input: Input) {
        //empty for now
    }
}

class Bar: Foo<Int> {
}

If I instantiate Foo, with its init method marked with @discardableResult, then the compiler does a good job of not warning me about the unused result.

However, if I instantiate Bar, who inherits its init method from Foo, I will get a warning:

Foo(input :10) //no warnings, nice!
Bar(input: 10) //warning: Result of 'Bar' initializer is unused

I believe this to be a bug and not a feature since Bar can and should inherit its init method from Foo to be able to get the same effect as Foo has.

@belkadan
Copy link
Contributor

belkadan commented May 2, 2018

I'm inclined to agree! Tagging as a StarterBug. The fix would be changing configureDesignatedInitAttributes in CodeSynthesis.cpp to test if the superclass constructor had a DiscardableResultAttr, and creating one for the new constructor as well if it does.

@vguerra
Copy link
Contributor

vguerra commented May 16, 2018

hello Vandad (JIRA User) and @belkadan . I took the liberty of assigning the bug to myself. I am new to the swift compiler and was looking at starter bugs I could help with and I think i can tackle this one 🙂.

@belkadan
Copy link
Contributor

Welcome, Victor! Feel free to open a pull request on GitHub as soon as you have something ready for initial review, or even if you just have questions as you go.

@vguerra
Copy link
Contributor

vguerra commented May 16, 2018

thanks Jordan!

I followed your prev. comment and I got a fix already. I am currently writing a test that makes sure the warning is not emitted but can't manage to execute it locally. Maybe you can give me a hand here 🙂.

I am launching the test as follows:

./llvm/utils/lit/lit.py -svv ./build/Xcode-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/attr --filter=attr_discardableResult.swift --param swift-version=4 -a

test fails with following stack trace:

Unable to find source-code formatter for language: python. Available languages are: actionscript, html, java, javascript, none, sql, xhtml, xmlTraceback (most recent call last):
  File "/Volumes/VicDrive/ML/swift-tensorflow/llvm/utils/lit/lit/run.py", line 202, in _execute_test_impl
    result = test.config.test_format.execute(test, lit_config)
  File "/Volumes/VicDrive/ML/swift-tensorflow/swift/test/swift_test.py", line 63, in execute
    result = super(SwiftTest, self).execute(test, litConfig)
  File "/Volumes/VicDrive/ML/swift-tensorflow/llvm/utils/lit/lit/formats/shtest.py", line 25, in execute
    self.execute_external)
  File "/Volumes/VicDrive/ML/swift-tensorflow/llvm/utils/lit/lit/TestRunner.py", line 1478, in executeShTest
    script = applySubstitutions(script, substitutions)
  File "/Volumes/VicDrive/ML/swift-tensorflow/llvm/utils/lit/lit/TestRunner.py", line 1178, in applySubstitutions
    return list(map(processLine, script))
  File "/Volumes/VicDrive/ML/swift-tensorflow/llvm/utils/lit/lit/TestRunner.py", line 1172, in processLine
    ln = re.sub(a, b, ln)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 155, in sub
    return _compile(pattern, flags).sub(repl, string, count)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 286, in _subx
    template = _compile_repl(template, pattern)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 271, in _compile_repl
    p = sre_parse.parse_template(repl, pattern)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/sre_parse.py", line 737, in parse_template
    s = Tokenizer(source)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/sre_parse.py", line 192, in __init__
    self.__next()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/sre_parse.py", line 194, in __next
    if self.index >= len(self.string):
TypeError: object of type 'NoneType' has no len()

i added some print statements in `TestRunner.py` to understand a bit more what is going on and I see that failure happens when applying substitution for : `%target-swift-reflection-test`

am I forgetting to set an environment variable? or maybe passing a parameter to the lit.py script?

Thank you for your help in advance!

@belkadan
Copy link
Contributor

Ah, for an Xcode build I believe you have to pass --param build_mode=RelWithDebInfo as well, since it's valid to use the generated Xcode project to do both Debug and RelWithDebInfo builds.

@belkadan
Copy link
Contributor

(Bug-to-file: have lit check if you're in an Xcode build and complain directly if so.)

@vguerra
Copy link
Contributor

vguerra commented May 16, 2018

thank you Jordan! That made it work.

about your last comment, do you want me to file a bug for that?

@belkadan
Copy link
Contributor

Heh, you're welcome to. I'll get to it if you don't.

@belkadan
Copy link
Contributor

SR-7714

@vguerra
Copy link
Contributor

vguerra commented May 17, 2018

thanks for creating SR-7714 🙂 .. it would be indeed a nice improvement.

@vguerra
Copy link
Contributor

vguerra commented May 18, 2018

@belkadan I created a PR for this (#16699 Could you please start smoke tests on it? Thanks!

@vguerra
Copy link
Contributor

vguerra commented May 22, 2018

PR that solves this issue has been merged.

@belkadan
Copy link
Contributor

Pulling into the 4.2 branch, since it's a small, safe change: #16796

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants