Thread overview
Problem with public example in Phobos
Dec 18, 2019
berni44
Dec 18, 2019
berni44
Dec 18, 2019
H. S. Teoh
December 18, 2019
I thought it's a fast fix, just adding some documentation and a unittest. But when running the style checker I got into trouble. A shortend version of the problem:

I added the following public unittest to writeUpToNextSpec in std.format:

    ///
    @safe unittest
    {
        import std.array;
        auto w = appender!(char[])();

        auto f = FormatSpec("input: %10d output: %s");
        assert(f.writeUpToNextSpec(w) == true);

        assert(w.data == "input: ");
        assert(f.spec == 'd');
        assert(f.width == 10);

        // [...]
    }

While it works with the normal unittest test routines (make -f posix.mak unittest) the stylechecker complains (make -f posix.mak style).

I meanwhile found out, that I have to add the template parameter dchar to FormatSpec, although I don't understand why. (I know, that these tests should work, when they are put into a separate file. But I do not understand why the original version doesn't work there.)

But more problematic: When I add this templete parameter, I still get an error from the style checker, this time it's a linker problem, and I'm completely lost here. The error does not occure when checking std.format, but when checking std.package:

parsing std/package.d
/usr/bin/ld: std_package.o: in function `_D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv':
__main.d:(.text._D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv[_D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv]+0x17): undefined reference to `_D3std6format__T10FormatSpecTwZQp6__initZ'
/usr/bin/ld: __main.d:(.text._D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv[_D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv]+0x38): undefined reference to `_D3std6format__T10FormatSpecTwZQp6__ctorMFNaNbNcNiNfxAwZSQCdQCc__TQByTwZQCe'
/usr/bin/ld: std_package.o: in function `_D3std6format__T10FormatSpecTwZQp__T17writeUpToNextSpecTSQCd5array__T8AppenderTAaZQnZQBxMFNaNlNfKQBpZb':
__main.d:(.text._D3std6format__T10FormatSpecTwZQp__T17writeUpToNextSpecTSQCd5array__T8AppenderTAaZQnZQBxMFNaNlNfKQBpZb[_D3std6format__T10FormatSpecTwZQp__T17writeUpToNextSpecTSQCd5array__T8AppenderTAaZQnZQBxMFNaNlNfKQBpZb]+0x15a): undefined reference to `_D3std6format__T10FormatSpecTwZQp6fillUpMFNaNlNfZv'


What do I miss?
December 18, 2019
On 12/18/19 3:56 AM, berni44 wrote:
> I thought it's a fast fix, just adding some documentation and a unittest. But when running the style checker I got into trouble. A shortend version of the problem:
> 
> I added the following public unittest to writeUpToNextSpec in std.format:
> 
>      ///
>      @safe unittest
>      {
>          import std.array;
>          auto w = appender!(char[])();
> 
>          auto f = FormatSpec("input: %10d output: %s");
>          assert(f.writeUpToNextSpec(w) == true);
> 
>          assert(w.data == "input: ");
>          assert(f.spec == 'd');
>          assert(f.width == 10);
> 
>          // [...]
>      }
> 
> While it works with the normal unittest test routines (make -f posix.mak unittest) the stylechecker complains (make -f posix.mak style).

style checker has nothing to do with buildable code. It just enforces the style follows the guidelines.

Just look at the output and see what it's asking you to do.

> 
> I meanwhile found out, that I have to add the template parameter dchar to FormatSpec, although I don't understand why. (I know, that these tests should work, when they are put into a separate file. But I do not understand why the original version doesn't work there.)

OK, so writeUpToNextSpec is a method inside a templated struct (FormatSpec). *Inside* any templated struct (or template of any kind really), the name of the template is synonymous with the *instantiated* template.

e.g.:

struct S(T)
{
   S *ptr;
}

static assert(is(typeof(S!int.ptr) == S!int *));

But when the code is copied *outside* the template, it needs the full instantiation.

This comes from the fact that unless you are declaring a template with `template`, you are using the eponymous template trick under the hood.

That is:

struct S(T)
{
}

is short for:

template S(T)
{
   struct S
   {
   }
}

where naturally the S inside the template is not the template but the struct.

In the case of a documented unittest, I think the CI is trying to make sure the test can run on its own.

Note, putting unittests inside templates duplicates the unit test for every instantiation. Using an instantiation other than the template itself is going to result in a lot of duplicated tests. Unfortunately, there is no mechanism to have a documented unittest that isn't part of the instantiation.

> 
> But more problematic: When I add this templete parameter, I still get an error from the style checker, this time it's a linker problem, and I'm completely lost here. The error does not occure when checking std.format, but when checking std.package:
> 
> parsing std/package.d
> /usr/bin/ld: std_package.o: in function `_D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv':
> __main.d:(.text._D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv[_D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv]+0x17): undefined reference to `_D3std6format__T10FormatSpecTwZQp6__initZ'
> /usr/bin/ld: __main.d:(.text._D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv[_D3std6format__T10FormatSpecTaZQp22__unittest_L1237_C11_1FNaNfZv]+0x38): undefined reference to `_D3std6format__T10FormatSpecTwZQp6__ctorMFNaNbNcNiNfxAwZSQCdQCc__TQByTwZQCe' 
> 
> /usr/bin/ld: std_package.o: in function `_D3std6format__T10FormatSpecTwZQp__T17writeUpToNextSpecTSQCd5array__T8AppenderTAaZQnZQBxMFNaNlNfKQBpZb': 
> 
> __main.d:(.text._D3std6format__T10FormatSpecTwZQp__T17writeUpToNextSpecTSQCd5array__T8AppenderTAaZQnZQBxMFNaNlNfKQBpZb[_D3std6format__T10FormatSpecTwZQp__T17writeUpToNextSpecTSQCd5array__T8AppenderTAaZQnZQBxMFNaNlNfKQBpZb]+0x15a): undefined reference to `_D3std6format__T10FormatSpecTwZQp6fillUpMFNaNlNfZv'

This might be a bug. I don't know how this is built, so I'm not sure. But it looks like the compiler isn't including some instantiation of FormatSpec when linking.

-Steve
December 18, 2019
On Wednesday, 18 December 2019 at 17:57:15 UTC, Steven Schveighoffer wrote:
> style checker has nothing to do with buildable code. It just enforces the style follows the guidelines.

It also checks, that the public unittests can be used on it's own. (Target publictests in the makefile. Didn't know that too until yesterday.)

> This comes from the fact that unless you are declaring a template with `template`, you are using the eponymous template trick under the hood.

Aaaah. It's so common, that one forgets about it. Thanks for reminding. :-)

> Note, putting unittests inside templates duplicates the unit test for every
> instantiation. Using an instantiation other than the template itself is going
> to result in a lot of duplicated tests. Unfortunately, there is no mechanism to have a documented unittest that isn't part of the instantiation.

I know about that problem. Would be good, to have some such mechanism. Until we have, I'll resort to moving them after the template and adding a link to the docs, see below.

> This might be a bug. I don't know how this is built, so I'm not sure. But it looks like the compiler isn't including some instantiation of FormatSpec when linking.

I moved the unittest outside the template, just to find out, that there is allready one, which is almost identical. I merged the two and added a link to the docs, so people can find the example.

Thanks a lot!

December 18, 2019
On Wed, Dec 18, 2019 at 07:16:15PM +0000, berni44 via Digitalmars-d-learn wrote:
> On Wednesday, 18 December 2019 at 17:57:15 UTC, Steven Schveighoffer wrote:
[...]
> > Note, putting unittests inside templates duplicates the unit test for every instantiation. Using an instantiation other than the template itself is going to result in a lot of duplicated tests. Unfortunately, there is no mechanism to have a documented unittest that isn't part of the instantiation.
> 
> I know about that problem. Would be good, to have some such mechanism. Until we have, I'll resort to moving them after the template and adding a link to the docs, see below.
[...]

Hackish workaround:

	struct TemplateStruct(T) {
		auto foo(...) { ... }

		static if (is(T == int))
			/// Look, ma! I'm documented!
			unittest {
			}
	}

	// Make sure the test actually runs
	version(unittest) alias dummy = TemplateStruct!int;


T

-- 
Why can't you just be a nonconformist like everyone else? -- YHL