posix.mak: module-specific -dip25/1000 for someModule.test and unitte… #6278

carblue · 2018-03-15T12:53:32Z

st rules

Somehow I managed to screw my git push-connection to closed PR #6195
So here it is again, updated.

This PR is not expected to be merged, possibly wrong to merge unchanged (I don't know; and it's useful temporarily only anyway).
"I'm not aware of the complete picture of the build process and all the consequences of such a phobos.posix.mak in CI: It may perhaps prevent merging a legitimate fix in one file because it touches yet non(-dip1000/-unit-)tested code of another phobos module, now failing due to -dip1000. "

It's main purpose is to document the state/infos/progress of -dip1000 compilable phobos and preferrably to be used locally by anybody interested. I will refer to this posix.mak in my "fix -dip1000 compilable" PRs.

It includes a global associative array 'aa' with module-specific (key) and value for -dip25/1000, according to how master's modules are compilable, updated regularly acc. progress.
Thus it's also the TODO-list for the "few" remaining -dip25 only compilable modules.
Members with write access can write into carblue:dip1000_3; I'll pay attention not to overwrite when pushing updates.
WIP_carblue will be my mark what I'm working on.
If there is a champion who wants to contribute for a difficult issue: It's the std.stdio. write family of functions, that needs fixing, currently triggering errors from several modules:
Just set -dip1000 for the aa[module] and go down to the rabbit hole.
I do these tests, each with -dip1000 and -dip25 as well (because it may happen to work for -dip1000, but not for -dip25 any more):
make -j6 -f posix.mak publictests and the next 2 ones

aa is included in 2 rules for unittesting/compiling, currently; suitable as is for Linux (at least):

make -f posix.mak path/to/someModule.test e.g. make -f posix.mak std/uni.test
make -j6 -f posix.mak unittest

I preferred associative array aa entries in the "modulename-notation" (e.g. aa[std.array]) instead of OS-dependant "pathname-notation" (didn't recall whether windows can cope with forward-slashes).
But this only defers the issue to the call-site (I use Linux, i.e. /): $(aa[$(subst /,.,$(basename $<))])

Used currently in 2 rules, e.g.
%.test : %.d $(LIB)
T=mktemp -d /tmp/.dmd-run-test.XXXXXX &&
(
$(DMD) -od$$T $(DFLAGS) $(aa[$(subst /,.,$(basename $<))]) -main $(UDFLAGS) $(LIB) -defaultlib= -debuglib= $(LINKDL) -cov -run $< ;
RET=$$? ; rm -rf $$T ; exit $$RET
)

Positioning $(aa[$(subst /,.,$(basename $<))]) after $(DFLAGS) resulting in e.g. -dip25 -dip1000 always worked for me, the compiler picked up the last one.

This way, the changes introduced by this PR may be completely removed again, once phobos is fully -dip1000 compilable, and simple changes like e.g. in PR 5915 (see also 5044) will do it.

dlang-bot · 2018-03-15T12:53:33Z

Thanks for your pull request and interest in making D better, @carblue! 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 annotated coverage diff directly on GitHub with CodeCov's browser extension
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.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

wilzbach · 2018-03-15T17:03:52Z

At the moment everything seems to error for me with:

std/uni.d(7458): Error: returning sliceOverIndexed(a, b, &this) escapes a reference to parameter this, perhaps annotate with return
std/uni.d(7464): Error: returning sliceOverIndexed(0LU, this.length(), &this) escapes a reference to parameter this, perhaps annotate with return
std/experimental/logger/filelogger.d(133): Error: @safe function std.experimental.logger.filelogger.FileLogger.beginLogMsg cannot call @system function std.experimental.logger.core.systimeToISOString!(LockingTextWriter).systimeToISOString
std/experimental/logger/filelogger.d(134): Error: @safe function std.experimental.logger.filelogger.FileLogger.beginLogMsg cannot call @system function std.format.formattedWrite!(LockingTextWriter, char, string, string, int).formattedWrite
std/experimental/logger/filelogger.d(143): Error: @safe function std.experimental.logger.filelogger.FileLogger.logMsgPart cannot call @system function std.format.formattedWrite!(LockingTextWriter, char, const(char)[]).formattedWrite
std/experimental/logger/filelogger.d(143): Error: scope variable msg assigned to non-scope parameter _param_2 calling std.format.formattedWrite!(LockingTextWriter, char, const(char)[]).formattedWrite

IIRC this is due to https://issues.dlang.org/show_bug.cgi?id=18445, but the stdx.logger bit would be fixed by #5915

wilzbach · 2018-03-15T17:05:12Z

/tmp/.dmd-run-test.3owix1/affix_allocator.o: In function `_D3std12experimental9allocator15building_blocks15bitmapped_block__T14BitmappedBlockVmi128Vki16TSQDqQDpQDeQCx14null_allocator13NullAllocatorZQCv14adjustStartIdxMFNaNbNiNfZv':
/home/circleci/phobos/std/experimental/allocator/building_blocks/bitmapped_block.d:251: undefined reference to `_D3std12experimental9allocator15building_blocks15bitmapped_block9BitVector3repMFNaNbNiNjNfZAm'
/home/circleci/phobos/std/experimental/allocator/building_blocks/bitmapped_block.d:252: undefined reference to `_D3std12experimental9allocator15building_blocks15bitmapped_block9BitVector3repMFNaNbNiNjNfZAm'
/tmp/.dmd-run-test.3owix1/affix_allocator.o: In function `_D3std12experimental9allocator15building_blocks15bitmapped_block__T14BitmappedBlockVmi128Vki16TSQDqQDpQDeQCx14null_allocator13NullAllocatorZQCv8allocateMFNbNexmZAv':
/home/circleci/phobos/std/experimental/allocator/building_blocks/bitmapped_block.d:317: undefined reference to `_D3std12experimental9allocator15building_blocks15bitmapped_block9BitVector3repMFNaNbNiNjNfZAm'
/home/circleci/phobos/std/experimental/allocator/building_blocks/bitmapped_block.d:319: undefined reference to `_D3std12experimental9allocator15building_blocks15bitmapped_block9BitVector3repMFNaNbNiNjNfZAm'
/home/circleci/phobos/std/experimental/allocator/building_blocks/bitmapped_block.d:323: undefined reference to `_D3std12experimental9allocator15building_blocks15bitmapped_block9BitVector3repMFNaNbNiNjNfZAm'
/tmp/.dmd-run-test.3owix1/affix_allocator.o:/home/circleci/phobos/std/experimental/allocator/building_blocks/bitmapped_block.d:324: more undefined references to `_D3std12experimental9allocator15building_blocks15bitmapped_block9BitVector3repMFNaNbNiNjNfZAm' follow
collect2: error: ld returned 1 exit status

I assume this is the -dip1000 ABI incompatibility issue which imho is a huge blocker and should raise red flags everywhere...

See also: #5915 (comment)

wilzbach

A few comments

wilzbach · 2018-03-15T16:47:06Z

posix.mak

@@ -340,12 +340,210 @@ endif
 $(addprefix $(ROOT)/unittest/,$(DISABLED_TESTS)) :
 	@echo Testing $@ - disabled

+# In particular, this posix.mak successfully compiles master (with module-specific settings acc. to aa) for:


This Makefile will also be on stable, so the comment doesn't make much sense.

wilzbach · 2018-03-15T16:49:45Z

posix.mak

+
+# global associative array aa:
+# keys: first dir-files, then sub-dir-files, lexical ordering
+# value: any dmd compiler switch, used here for -dip25/1000 only


I would drop the entire comment and just say: "Module-specific compiler switches (needed for the -dip1000 transition)"
However, if you want please feel free to improve the header of this file (e.g. if it was hard for you to figure out how to run the test for an individual module or overall), but it's a redundant here.

Thanks for Your review, Seb. Agreed. Since I'm more wordy in the PR introducing comment, that's redundant. Carsten

wilzbach · 2018-03-15T16:53:03Z

posix.mak

+aa[std.process]=-dip1000
+aa[std.random]=-dip1000
+aa[std.signals]=-dip1000
+aa[std.socket]=-dip25 # depends on PR 6204 merged, which will be at least deferred or possibly rejected (deprecation process required due to changed class Socket)


Hmm #6204 was closed :/

Reopened that, FWIW.

wilzbach · 2018-03-15T16:55:55Z

posix.mak

+aa[std.algorithm.iteration]=-dip25 #    WIP
+aa[std.algorithm.mutation]=-dip1000
+aa[std.algorithm.package]=-dip1000
+aa[std.algorithm.searching]=-dip25 # depends on PR 6246 merged and std.algorithm.comparison fixed


#6246 was closed.
I get the following for pmake std/algorithm/searching.test DFLAGS=-dip1000 so

std/uni.d(7458): Error: returning sliceOverIndexed(a, b, &this) escapes a reference to parameter this, perhaps annotate with return std/uni.d(7464): Error: returning sliceOverIndexed(0LU, this.length(), &this) escapes a reference to parameter this, perhaps annotate with return std/experimental/logger/filelogger.d(133): Error: @safe function std.experimental.logger.filelogger.FileLogger.beginLogMsg cannot call @system function std.experimental.logger.core.systimeToISOString!(LockingTextWriter).systimeToISOString std/experimental/logger/filelogger.d(134): Error: @safe function std.experimental.logger.filelogger.FileLogger.beginLogMsg cannot call @system function std.format.formattedWrite!(LockingTextWriter, char, string, string, int).formattedWrite std/experimental/logger/filelogger.d(143): Error: @safe function std.experimental.logger.filelogger.FileLogger.logMsgPart cannot call @system function std.format.formattedWrite!(LockingTextWriter, char, const(char)[]).formattedWrite std/experimental/logger/filelogger.d(143): Error: scope variable msg assigned to non-scope parameter _param_2 calling std.format.formattedWrite!(LockingTextWriter, char, const(char)[]).formattedWrite

the logger part is fixed by #5915

So it depends on https://issues.dlang.org/show_bug.cgi?id=18445 + #5915?

I did reopen and update #6246
There is a strange new error there: Error: struct std.range.SortedRange!(int[], "a < b").SortedRange member _input is not accessible from @safe code

I'm going through all modules once more (someModule.test) and prepare an overall comment and update for this PR, trying to get synchronized with Your findings.

wilzbach · 2018-03-15T16:58:46Z

posix.mak

+aa[std.algorithm.package]=-dip1000
+aa[std.algorithm.searching]=-dip25 # depends on PR 6246 merged and std.algorithm.comparison fixed
+aa[std.algorithm.setops]=-dip1000
+aa[std.algorithm.sorting]=-dip25 # i.a. depends on (fixed issue 17512 available in master and) a fix for writefln


(I get the same error as for searching) so depends on std.algorithm.searching?

wilzbach · 2018-03-15T17:00:53Z

posix.mak

+aa[std.experimental.allocator.building_blocks.segregator]=-dip1000
+aa[std.experimental.allocator.building_blocks.stats_collector]=-dip1000
+aa[std.experimental.logger.core]=-dip1000       # PR 6266
+aa[std.experimental.logger.filelogger]=-dip25 # PR 6266; depends on a fix for: std.format.formattedWrite


wilzbach · 2018-03-15T17:01:41Z

posix.mak

+aa[std.net.isemail]=-dip1000
+
+aa[std.range.interfaces]=-dip1000
+aa[std.range.package]=-dip25 # i.a. depends on the issue 17512 fix available in master


Depends on https://issues.dlang.org/show_bug.cgi?id=18445 + #5915 for me

wilzbach · 2018-03-15T17:02:30Z

posix.mak

+aa[std.regex.internal.kickstart]=-dip1000
+aa[std.regex.internal.parser]=-dip1000
+aa[std.regex.internal.tests2]=-dip1000
+aa[std.regex.internal.tests]=-dip25 # depends on a fix for writeln


Depends on https://issues.dlang.org/show_bug.cgi?id=18445 + #5915 for me

JackStouffer · 2018-03-15T20:22:03Z

See the change in #6281

We need a way to test specific DIP1000 changes to @safe. I just choose using version as a possible option. Thoughts?

carblue · 2018-03-17T13:00:24Z

@wilzbach This comment is going to be huge, thus here is part 1:
This is funny, I just learned: In english, it's "compare apples and oranges" whereas german speakers say "compare apples and pears". It's not easy to mix colour of apples and oranges and also not easy to mix shape of apples and pears. So the Brit must have been color-blind and the German long-sighted when having comparing problems in front of them?

In this comment I refer to the following sources:

phobos:   Latest commit a5eb8e2 8 hours ago; dlang-bot Merge pull request #6287 from wilzbach/fix-changelog ; Add .dd extension to the fixDigitGrouping changelog entry ; merged-on-behalf-of: Jack Stouffer <jack@jackstouffer.com>
druntime: Latest commit f2711a3 2 days ago;  andralex  Merge pull request #2142 from WalterBright/object-escape ; object: fix escaping p
dmd:      Latest commit aa1b363 9 hours ago; wilzbach  Merge pull request #7817 from wilzbach/switch-skip-decl ; End deprecation phase for `switch` cases which skip the declaration of a variable

Now it's the first time I will have to "step back" from -dip1000 to -dip25 in posix.mak for: std.exception, due to https://issues.dlang.org/show_bug.cgi?id=18478.
I know You intended to prevent exactly that by this posix.mak; in this case it is probably caused by dmd, a commit related to https://issues.dlang.org/show_bug.cgi?id=17512 ?
There are other modules like affix_allocator where I will have to/should? "step back" from -dip1000 to -dip25 due to linker errors, my fault in the first place? (possibly not checked individually by make -f posix.mak path/to/someModule.test).

For me (Linux x86_64), with that "aa[std.exception]=-dip25" change to #6278 (and some comming comment changes) I get these results:

make -j6 -f posix.mak unittest <= PASS everywhere
make -j6 -f posix.mak publictests <= no errors
make -f posix.mak std/uni.test <= no errors

With aa[std.file]=-dip1000 temporarily for the next 3 tests, dir phobos/generated erased before each run:
make -j6 -f posix.mak unittest <= PASS everywhere
make -j6 -f posix.mak publictests <= no errors
make -f posix.mak std/file.test <= linker error :

/tmp/.dmd-run-test.BKXrI1/file.o: In Funktion `_D3std3uni__T9normalizeVEQxQv17NormalizationFormi0TaZQBoFNkANgaZ14__foreachbody2MFNfKwZi':
__main.d:(.text._D3std3uni__T9normalizeVEQxQv17NormalizationFormi0TaZQBoFNkANgaZ14__foreachbody2MFNfKwZi[_D3std3uni__T9normalizeVEQxQv17NormalizationFormi0TaZQBoFNkANgaZ14__foreachbody2MFNfKwZi]+0x67): Nicht definierter Verweis auf (undefined reference) `_D3std3uni__T16SliceOverIndexedTSQBfQBe8GraphemeZQBk7opSliceMFNaNbNiNjNfZSQCuQCt__TQCsTQCdZQDa'
collect2: error: ld returned 1 exit status
Error: linker exited with status 1
posix.mak:581: die Regel für Ziel „std/file.test“ scheiterte (the rule for target „std/file.test“ failed)

The demangled undefined reference is: pure nothrow @nogc return @safe std.uni.SliceOverIndexed!(std.uni.Grapheme).SliceOverIndexed std.uni.SliceOverIndexed!(std.uni.Grapheme).SliceOverIndexed.opSlice()
Source's mem fun definition is: @safe struct Grapheme. SliceOverIndexed!Grapheme opSlice() @nogc nothrow pure return
I assume, the reason is that the phobos libs are compiled with -dip25 for all modules currently. Probably the posix.mak has to be adapted to fix this linker error.

make -f posix.mak std/utf.test <= no errors
For the next 3 tests, temporarily #5915 applied, dir phobos/generated erased before each run:
make -j6 -f posix.mak unittest <= PASS everywhere
make -j6 -f posix.mak publictests <= no errors
make -f posix.mak std/utf.test <= no errors

carblue · 2018-03-19T08:09:11Z

@wilzbach
Commit dbdbc21 is verified to compile with Linux x86_64 (for me) and
make -j6 -f posix.mak unittest
make -j6 -f posix.mak publictests
under these conditions:

dmd : Latest commit 17bc56c
druntime: Latest commit f2711a3
phobos : Latest commit 6947d80

and requiring this change applied (#6295):

diff --git a/std/container/slist.d b/std/container/slist.d
index 8c2f2fc..04c4d73 100644
--- a/std/container/slist.d
+++ b/std/container/slist.d
@@ -354,7 +354,7 @@ Returns: The number of elements inserted
 
 Complexity: $(BIGOH m), where $(D m) is the length of $(D stuff)
      */
-    size_t insertFront(Stuff)(Stuff stuff)
+    size_t insertFront(Stuff)(scope Stuff stuff)
     if (isInputRange!Stuff && isImplicitlyConvertible!(ElementType!Stuff, T))
     {
         initialize();

There are some comments containing DROP
where I had to step back from -dip1000 to -dip25, probably due to some recent source changes.

wilzbach

Ok. I am still on the road for a few hours, but we should merge this ASAP to avoid further regressions, so I might push to your PR later.

wilzbach · 2018-03-19T14:31:42Z

posix.mak

+aa[std.ascii]=-dip1000
+aa[std.base64]=-dip1000
+aa[std.bigint]=-dip1000
+aa[std.bitmanip]=-dip1000 # PR 6174


Walter is a huge fan of clickable links

I'm a fan of efficiency (including clickable links; sorry, in a hurry I sometimes forget them).
In this case I don't know if they work from there, but I'll update to URLs.

carblue · 2018-03-19T16:19:06Z

@wilzbach
Commit a606697 is verified to compile with Linux x86_64 (for me) and
make -j6 -f posix.mak unittest
make -j6 -f posix.mak publictests

under these conditions:
dmd : Latest commit bbe466b97fe5d2fa9f2914937f42079a987579d2
druntime: Latest commit f2711a31830260eec5710daa2bd54090d7bbe7f8
phobos : Latest commit 606351f

and requiring a slist.d fix like #6295 or the outdated one above.

wilzbach · 2018-03-19T19:29:39Z

Set the failing std.algorithm.mutation test to -dip25 until std.container.slist is DIP1000 again
Squashed your commits

…st rules

…t being not-compatible yet

wilzbach · 2018-03-19T23:29:40Z

Update: @CyberShadow had the excellent idea to move these changes to a separate makefile - I have done so for you.

wilzbach · 2018-03-20T00:29:47Z

OK now we should finally be able to make incremental progress on dip1000 :)

carblue requested review from CyberShadow, MartinNowak and wilzbach as code owners March 15, 2018 12:53

wilzbach mentioned this pull request Mar 15, 2018

Compile Phobos with -dip1000 #5915

Closed

wilzbach reviewed Mar 15, 2018

View reviewed changes

wilzbach mentioned this pull request Mar 16, 2018

Work On Issue 18172 - add scope to pointers in getopt to allow @safe #6281

Closed

carblue mentioned this pull request Mar 17, 2018

std.algorithm.searching: Fix a -dip1000 compilable issue #6246

Merged

carblue mentioned this pull request Mar 19, 2018

[Trivial]std.uni: Fix a -dip1000 compilable issue; appeared since #6041 #6294

Merged

wilzbach approved these changes Mar 19, 2018

View reviewed changes

JackStouffer added this to In progress in DIP1000 via automation Mar 19, 2018

carblue requested a review from JackStouffer as a code owner March 19, 2018 19:30

carblue and others added 2 commits March 19, 2018 23:49

posix.mak: module-specific -dip25/1000 for someModule.test and unitte…

7a7d37f

…st rules

Disable std.algorithm.mutation for -dip1000 due to std.container.slis…

c280f45

…t being not-compatible yet

wilzbach added the auto-merge label Mar 19, 2018

Move -di1000 mapping to a separate file

e51c4d8

dlang-bot removed the auto-merge label Mar 19, 2018

wilzbach added the auto-merge label Mar 19, 2018

wilzbach merged commit 4f42f45 into dlang:master Mar 20, 2018

DIP1000 automation moved this from In progress to Done Mar 20, 2018

wilzbach mentioned this pull request Apr 9, 2018

[BLOCKED][WIP] Don't instantiate unittest templates in libraries dlang/dmd#8124

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

posix.mak: module-specific -dip25/1000 for someModule.test and unitte… #6278

posix.mak: module-specific -dip25/1000 for someModule.test and unitte… #6278

carblue commented Mar 15, 2018

dlang-bot commented Mar 15, 2018

wilzbach commented Mar 15, 2018

wilzbach commented Mar 15, 2018

wilzbach left a comment

wilzbach Mar 15, 2018

wilzbach Mar 15, 2018

carblue Mar 15, 2018

wilzbach Mar 15, 2018

carblue Mar 15, 2018

wilzbach Mar 15, 2018

carblue Mar 17, 2018

wilzbach Mar 15, 2018

wilzbach Mar 15, 2018

wilzbach Mar 15, 2018

wilzbach Mar 15, 2018

JackStouffer commented Mar 15, 2018

carblue commented Mar 17, 2018

carblue commented Mar 19, 2018

wilzbach left a comment

wilzbach Mar 19, 2018

carblue Mar 19, 2018

carblue commented Mar 19, 2018

wilzbach commented Mar 19, 2018

wilzbach commented Mar 19, 2018

wilzbach commented Mar 20, 2018

posix.mak: module-specific -dip25/1000 for someModule.test and unitte… #6278

posix.mak: module-specific -dip25/1000 for someModule.test and unitte… #6278

Conversation

carblue commented Mar 15, 2018

dlang-bot commented Mar 15, 2018

Bugzilla references

wilzbach commented Mar 15, 2018

wilzbach commented Mar 15, 2018

wilzbach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JackStouffer commented Mar 15, 2018

carblue commented Mar 17, 2018

carblue commented Mar 19, 2018

wilzbach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carblue commented Mar 19, 2018

wilzbach commented Mar 19, 2018

wilzbach commented Mar 19, 2018

wilzbach commented Mar 20, 2018