fix Issue 17419 - add __traits(getLinkage, s) to the the linkage of s… #6822
Conversation
|
| return new ErrorExp(); | ||
| } | ||
| string linkage; | ||
| switch (d.linkage) |
There was a problem hiding this comment.
.def is not represented
There was a problem hiding this comment.
What is .def ?
give it a string representation, then.
Better then a halt.
There was a problem hiding this comment.
ahh default linkage.
This should be D linkage, no ?
|
Doesn't this need an accompanying PR to dlang.org for the doc change and probably a changelog entry as well? |
| case LINK.system: linkage = "System"; break; | ||
| default: assert(0); | ||
| } | ||
| auto se = new StringExp(e.loc, cast(char*)linkage.ptr); |
There was a problem hiding this comment.
also maybe use new StringExp(e.loc, cast(void*) &linkage[0], linkage.length ?
There was a problem hiding this comment.
I don't see the point.
There was a problem hiding this comment.
the current version will call strlen inside of StringExp constructor.
which is probably negligible but still we don't need to throw the length information away.
There was a problem hiding this comment.
Not worth the extra complexity.
| } | ||
| string linkage; | ||
| switch (d.linkage) | ||
| { |
There was a problem hiding this comment.
Maybe ... rather then using strings here we take the pointers to the identifiers out of the string table we already have ?
Id.{D, C, Windows ... }.string
There was a problem hiding this comment.
Won't work because of C++ and Objective-C
test/compilable/test17419.d
Outdated
| extern (Objective-C) int fooobjc(); | ||
| extern (System) int foos(); | ||
|
|
||
| static assert(__traits(getLinkage, fooc) == "C"); |
There was a problem hiding this comment.
I suppose this was to be food ?
There was a problem hiding this comment.
the symbol food, is not referenced in this test.
while fooc is referenced twice.
test/compilable/test17419.d
Outdated
| static assert(__traits(getLinkage, fooc) == "C"); | ||
| static assert(__traits(getLinkage, aliasc) == "C"); | ||
|
|
||
| extern (C) int food(); |
| case LINK.windows: linkage = "Windows"; break; | ||
| case LINK.pascal: linkage = "Pascal"; break; | ||
| case LINK.objc: linkage = "Objective-C"; break; | ||
| case LINK.system: linkage = "System"; break; |
There was a problem hiding this comment.
This line is uncovered ...
I guess system linkage is replaced earlier
should this be an ice then ?
There was a problem hiding this comment.
Extra complexity of duplicating other logic is not worth it.
There was a problem hiding this comment.
extern(System) is resolved when building the parsing tree. If anyone wants to fix that to defer until semantic, be my guest. However I suspect even then that is still before traits are evaluated.
| case LINK.pascal: linkage = "Pascal"; break; | ||
| case LINK.objc: linkage = "Objective-C"; break; | ||
| case LINK.system: linkage = "System"; break; | ||
| default: assert(0); |
There was a problem hiding this comment.
please consider making this a proper ice
There was a problem hiding this comment.
No, that's what asserts are for.
…ymbol s
The use of name mangling in std.traits is slow, unreliable and brittle. Should be replaced with __traits(). This is a start.