Skip to content

Print the platform probe atomically#1796

Merged
PetarKirov merged 2 commits intodlang:masterfrom
SSoulaimane:fixprobe
Nov 11, 2019
Merged

Print the platform probe atomically#1796
PetarKirov merged 2 commits intodlang:masterfrom
SSoulaimane:fixprobe

Conversation

@SSoulaimane
Copy link
Member

DMD may print some stuff in between calls of pragma(msg) and corrupt the json data.

This is blocking DMD PR dlang/dmd#10539, all buildkite tests which depend on dub are failing.

After debugging, I found out It's printing a probe like this:

__dub_probe_begin__
{
  "compiler": "dmd",
  "frontendVersion": 2088,
  "compilerVendor": "Digital Mars D",
  "platform": [
import    core.internal.array.utils	(/home/ss/Downloads/dmd/generated/linux/debug/64/../../../../../druntime/import/core/internal/array/utils.d)
    "linux", "posix"
  ],
  "architecture": [
    "x86_64"
   ],
}
__dub_probe_end__

It fails at parsing the probe as json and aborts everything.

DMD may print some stuff in between call of `pragma(msg)` and corrupt the json data.
@dlang-bot
Copy link
Collaborator

Thanks for your pull request and interest in making D better, @SSoulaimane! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

@WebFreak001
Copy link
Member

this code was extremely difficult to read before and after. Now with even more differently formatted string literals inside that string literal it's confusing.

Can we maybe change parts of the generated code to `` strings or q"EOS <multiple lines> EOS" for this big block where it's not obvious at a first glance which parts are "strings outside the strings", "strings inside the strings", "code inside the strings" or which parts are "code outside the strings"

@wilzbach
Copy link
Contributor

this code was extremely difficult to read before and after. Now with even more differently formatted string literals inside that string literal it's confusing.

The proper solution is to use the new -Xi=compilerInfo anyhow, so I don't see any problems with this as the only maintainance step should be to remove this.

@SSoulaimane
Copy link
Member Author

SSoulaimane commented Nov 10, 2019

@wilzbach dub has to support older compiler versions, so I don't think this method will be replaced by anything recent.

@wilzbach
Copy link
Contributor

dub has to support older compiler versions, so I don't think a this method will be replaced by anything recent.

Well, the last ten releases have this new interface which is incidentally also what we officially support as the lowest bootstrap compiler, so that's not really a big concern.

Anyhow, my point was that there's no point in investing more time into making this old interface look nicer as it's maintainance only.

@SSoulaimane
Copy link
Member Author

SSoulaimane commented Nov 10, 2019

@WebFreak001 it looks fine to me, the only confusing bit is to figure out the q{ } where it starts and ends. maybe std.string.format() can be applied.

@PetarKirov PetarKirov merged commit f87302d into dlang:master Nov 11, 2019
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.

5 participants