Fix version with NULLPTR in JSON output #11640
Conversation
wilzbach
requested review from
Geod24,
ibuclaw,
MartinNowak,
rainers,
UplinkCoder and
WalterBright
as code owners
|
Thanks for your pull request, @wilzbach! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#11640" |
wilzbach
force-pushed
the
_version
branch
3 times, most recently
from
c35d69d
to
5c58089
Compare
src/dmd/globals.d
Outdated
| @@ -316,7 +316,8 @@ extern (C++) struct Global | |||
| Array!(const(char)*)* path; // Array of char*'s which form the import lookup path | |||
| Array!(const(char)*)* filePath; // Array of char*'s which form the file import lookup path | |||
|
|
|||
| string _version; | |||
| enum string _version = import("VERSION"); | |||
| enum uint _versionNumber = calcVersionNumber(_version); | |||
There was a problem hiding this comment.
Aren't enums only existent at CT and not passed to the layout?
There was a problem hiding this comment.
Aren't enums only existent at CT and not passed to the layout?
Yes. But there is still the field in the headers.
There was a problem hiding this comment.
Fair enough. I set _version to private, removed it from the header and exposed it to C++ via versionString and added a small test to cxx-frontend. I hope that works for GDC?
|
Great, so there's https://issues.dlang.org/show_bug.cgi?id=21206 (and the entire mess of arrays/string interop with C++). Example: extern(C++):
struct Foo {
string s;
}
void foo(Foo s) {} // OK
string foo() { return ""; } // OK on Posix, Error on Windows
void foo(string s) {} // errorhttps://run.dlang.io/is/FaSmWk Generated header file: // Automatically generated by Digital Mars D Compiler v2093
#pragma once
#include <stddef.h>
#include <stdint.h>
struct Foo;
struct Foo
{
DArray< char > s;
Foo() : s() {}
};
extern void foo(Foo s);
extern DArray< char > foo();
extern void foo(DArray< char > s); |
|
Could you make the |
Done. |
MoonlightSentinel
added
the
72h no objection -> merge
|
Blocked by https://issues.dlang.org/show_bug.cgi?id=21217 -> #11679 |
src/dmd/frontend.h
Outdated
| DArray< char > _version; | ||
| ENUM_CONSTANT(const char*, _version, "v2.093.1-503-g4c81d4e38") | ||
|
|
||
| ENUM_CONSTANT_NUMERIC(uint32_t, _versionNumber, 2093u) |
There was a problem hiding this comment.
do you plan to leave this hardcoded?
There was a problem hiding this comment.
See my prior comment - https://issues.dlang.org/show_bug.cgi?id=21217 and #11679.
There was a problem hiding this comment.
leave this hardcoded?
It's gone now ;-)
global._version has been changed at some point to an null-terminated string. However, its type was still a D string. Some parts of the code base were updated to do `.length - 1`, but other like the `compilerInfo` were forgotten. This change remedies this and: - removes null terminator from the D string of _version - initializes versionNumber at compile-time - expose _version as versionString (D) and versionChars (C++)
MoonlightSentinel
added
auto-merge
and removed
72h no objection -> merge
global._version has been changed at some point to an
null-terminated string. However, its type was still a D string. Some
parts of the code base were updated to do
.length - 1, but other likethe
compilerInfowere forgotten. This change remedies this and:_versionasversionStringCC @MoonlightSentinel