Skip to content

Fix a few more cases of ArrayRef problems. #5210

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

Merged
merged 1 commit into from
Oct 10, 2016

Conversation

bob-wilson
Copy link
Contributor

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

This is another problem (like 5356cc3) exposed by building with a newer
version of Clang. Construct a temporary SmallVector when needed to create
an ArrayRef.

This is another problem (like 5356cc3) exposed by building with a newer
version of Clang. Construct a temporary SmallVector when needed to create
an ArrayRef.
@bob-wilson
Copy link
Contributor Author

@swift-ci please smoke test

@jrose-apple
Copy link
Contributor

This one I would expect to work correctly as is. How did you find the issue?

@bob-wilson bob-wilson merged commit a4dd11c into swiftlang:master Oct 10, 2016
@bob-wilson bob-wilson deleted the array-ref-fix branch October 10, 2016 16:38
@bob-wilson
Copy link
Contributor Author

I just ran it in the debugger and it crashed while iterating over the array. Since it looked like the other problems, I just blindly tried the same fix.

@jrose-apple
Copy link
Contributor

Hm. I'm looking at the AST dump and it looks like it should be okay. It's the moral equivalent of

extern int &fallback();
int p = getValue();
const int &r = p > 0 ? p : fallback();
use(r);

and I'm pretty sure Clang reusing the stack space for p would be a bug.

@jrose-apple
Copy link
Contributor

(Ignore the lifetime extension rules for temporaries here. p itself is not a temporary.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants