Skip to content

Fix compilation with DIP1000 and compatibility with future in=scope const dmd change #1791

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.

Sign up for GitHub

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
wilzbach merged 4 commits into from
Nov 1, 2019
Merged

Fix compilation with DIP1000 and compatibility with future in=scope const dmd change #1791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99b085c
[scope] [Dependency] Don't annotate constructor parameter with scope
PetarKirov Oct 31, 2019
47f0452
[build.sh] Use DFLAGS env variable to allow customizing the D compile…
PetarKirov Oct 31, 2019
cf34c86
[.travis.yml]: Bump latest test compiler to dmd-2.088.1
PetarKirov Oct 31, 2019
56307b3
[dip1000] Fix compilation and test it on Travis CI
PetarKirov Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ jobs:
- d: gdc
include:
- stage: test
d: dmd-2.086.0
env: [FRONTEND=2.086]
d: dmd-2.088.1
env: [FRONTEND=2.088]
- d: dmd-2.081.1
env: [FRONTEND=2.081]
- d: dmd-2.080.1
Expand Down
3 changes: 2 additions & 1 deletion build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ fi
MACOSX_DEPLOYMENT_TARGET=10.8

echo Running $DMD...
$DMD -ofbin/dub -g -O -w -version=DubUseCurl -version=DubApplication -Isource $* @build-files.txt
DFLAGS="${DFLAGS:--g -O -w}"
$DMD -ofbin/dub $DFLAGS -version=DubUseCurl -version=DubApplication -Isource $* @build-files.txt
bin/dub --version
echo DUB has been built as bin/dub.
echo
Expand Down
2 changes: 1 addition & 1 deletion source/dub/compilers/dmd.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ config /etc/dmd.conf
case TargetType.executable:
if (platform.platform.canFind("windows"))
return settings.targetName ~ ".exe";
else return settings.targetName;
else return settings.targetName.idup;
Copy link
Contributor

@MoonlightSentinel MoonlightSentinel Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this copy be avoided by annotating settings with return?

Copy link
Member Author

@PetarKirov PetarKirov Nov 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that should work and I thought about that, but I decided not to change the interface. I think it's more of compiler deficiency that it doesn't recognize that settings.targetName in GC allocated and so it has infinite lifetime, and so it's safe to escape it.

Copy link
Member Author

@PetarKirov PetarKirov Nov 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I now remember why I didn't add the return attribute. The reason is that in all, but this single case, this function doesn't escape any part of it's parameter. Although this may sound too theoretical, in principle adding return attribute essentially limits callers of the function, meaning that just because of this single case they will need ensure that lifetime of settings is as long as the one of the result, which is mildly annoying.
A better solution than the idup is a function (with potentially the same name) that returns the argument as it is (without copying) if it is immutable with infinite lifetime.

case TargetType.library:
case TargetType.staticLibrary:
if (platform.platform.canFind("windows"))
Expand Down
2 changes: 1 addition & 1 deletion source/dub/compilers/gdc.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class GDCCompiler : Compiler {
case TargetType.executable:
if (platform.platform.canFind("windows"))
return settings.targetName ~ ".exe";
else return settings.targetName;
else return settings.targetName.idup;
case TargetType.library:
case TargetType.staticLibrary:
return "lib" ~ settings.targetName ~ ".a";
Expand Down
2 changes: 1 addition & 1 deletion source/dub/compilers/ldc.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ config /etc/ldc2.conf (x86_64-pc-linux-gnu)
return settings.targetName ~ ".exe";
else if (p.canFind("wasm"))
return settings.targetName ~ ".wasm";
else return settings.targetName;
else return settings.targetName.idup;
case TargetType.library:
case TargetType.staticLibrary:
if (p.canFind("windows") && !p.canFind("mingw"))
Expand Down
8 changes: 4 additions & 4 deletions source/dub/dependency.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ struct Dependency {
/** Constructs a new dependency specification that matches a specific
version.
*/
this(in Version ver)
this(const Version ver)
{
m_inclusiveA = m_inclusiveB = true;
m_versA = ver;
Expand Down Expand Up @@ -373,7 +373,7 @@ struct Dependency {
These methods are suitable for equality comparisons, as well as for
using `Dependency` as a key in hash or tree maps.
*/
bool opEquals(in Dependency o)
bool opEquals(const Dependency o)
const {
// TODO(mdondorff): Check if not comparing the path is correct for all clients.
return o.m_inclusiveA == m_inclusiveA && o.m_inclusiveB == m_inclusiveB
Expand All @@ -382,7 +382,7 @@ struct Dependency {
}

/// ditto
int opCmp(in Dependency o)
int opCmp(const Dependency o)
const {
if (m_inclusiveA != o.m_inclusiveA) return m_inclusiveA < o.m_inclusiveA ? -1 : 1;
if (m_inclusiveB != o.m_inclusiveB) return m_inclusiveB < o.m_inclusiveB ? -1 : 1;
Expand Down Expand Up @@ -762,7 +762,7 @@ struct Version {
return compareVersions(m_version, other.m_version);
}
/// ditto
int opCmp(in Version other) const { return opCmp(other); }
int opCmp(const Version other) const { return opCmp(other); }

/// Returns the string representation of the version/branch.
string toString() const { return m_version; }
Expand Down
4 changes: 2 additions & 2 deletions source/dub/internal/vibecompat/core/file.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ struct RangeFile {
@safe:
std.stdio.File file;

void put(in ubyte[] bytes) @trusted { file.rawWrite(bytes); }
void put(in char[] str) { put(cast(const(ubyte)[])str); }
void put(scope const ubyte[] bytes) @trusted { file.rawWrite(bytes); }
void put(scope const char[] str) { put(cast(const(ubyte)[])str); }
void put(char ch) @trusted { put((&ch)[0 .. 1]); }
void put(dchar ch) { char[4] chars; put(chars[0 .. encode(chars, ch)]); }

Expand Down
4 changes: 4 additions & 0 deletions travis-ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ if [ -z "$FRONTEND" -o "$FRONTEND" \> 2.074.z ]; then
dub test --compiler=${DC} -c library-nonet
fi

if [ "$FRONTEND" \> 2.087.z ]; then
DFLAGS='-preview=dip1000 -w -g -debug' DMD="$(command -v $DMD)" ./build.sh
fi

function clean() {
# Hard reset of the DUB local folder is necessary as some tests
# currently don't properly clean themselves
Expand Down