-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Don't report compile-time errors for promoteds #49779
Conversation
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
e0023ef
to
1f8d0be
Compare
Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1f8d0be
to
ec8ec97
Compare
so... this has amplified the breaking change for constants with errors (#49765). Before it was allowed to declare a This is a breaking change. Making these errors be warnings instead would remove all errors from constants and move them to the use-site. I don't think we can do this in a more fine-grained way. We could make the warning |
ec8ec97
to
590e636
Compare
I simplified this PR to the bare minimum, so only the promoted-changes |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
590e636
to
95946e1
Compare
@bors r+ |
📌 Commit 95946e1 has been approved by |
💔 Test failed - status-travis |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
95946e1
to
8de9fd4
Compare
I cannot reproduce locally (rebased) and Travis cannot reproduce either. Not sure what's going on. |
Let's retry. We need this for beta @bors r=eddyb |
📌 Commit a406af8 has been approved by |
⌛ Testing commit a406af8 with merge e5b78d9b47703363d7af20c411b3613622ada7ec... |
💔 Test failed - status-travis |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
So... it looks like this is due to optimization levels changing the output. (only happens on the noopt builder) Since the runtime behaviour would also differ (panic/no-panic), is it fine if the diagnostics differ? I'd just mark the test as always running with optimizations |
Hey wanted to circle back to this as it's beta-nominated and wanted to make sure it landed in a timely fashion for the next release, @oli-obk was that last question all that's blocking this? I'd imagine that for fixing a regression that such behavior is probably fine for now. |
The error messages differ between optimized and nonoptimized mode
@bors r=eddyb |
📌 Commit bb367c4 has been approved by |
☀️ Test successful - status-appveyor, status-travis |
Fixes the regression part of #49760, the missing warnings still are missing
r? @eddyb