rejectedsoftware / diet-ng

Compile-time indentation based, XML structured template system
MIT License
45 stars 24 forks source link

Fix visibility attribute #62

Closed RazvanN7 closed 5 years ago

RazvanN7 commented 5 years ago

This is currently blocking [1]. It looks like the struct is declared as package, but I don't see a package defined anywhere so the access modifier will default to private, thus making this line [2] an error. Probably creating a package hierarchy would be a more elegant fix, but I am not sufficiently familiar with codebase to do that and simply grepping for package reveals that this is the single occurrence of it.

Please push a new tag for diet-ng after this is merged, so that the CI uses the latest verrsion.

Thanks!

[1] https://github.com/dlang/dmd/pull/9393 [2] https://github.com/rejectedsoftware/diet-ng/blob/master/source/diet/input.d#L7

RazvanN7 commented 5 years ago

Ping

s-ludwig commented 5 years ago

Hm, I must be missing something very fundamental here. Why would diet not be considered as the package, when the modules are diet.input and diet.traits? I don't think this is the only place where I'm using package this way.

wilzbach commented 5 years ago

I think package(diet) would be required for this, package is by default only for the current package :/

s-ludwig commented 5 years ago

So what is the "current" package? What was it before the "package.d" feature was added?

RazvanN7 commented 5 years ago

The current package defaults to the module in which the declaration resides, if no package is defined. For more information check [1] [2]

[1] https://dlang.org/spec/attribute.html#visibility_attributes [2] https://dlang.org/spec/module.html#package-module

s-ludwig commented 5 years ago

Quoting from the first refrence:

package extends private so that package members can be accessed from code in other modules that are in the same package. If no identifier is provided, this applies to the innermost package only, or defaults to private if a module is not nested in a package.

It seems pretty clear to me from this that package in the module diet.traits should make the declaration accessible from all diet.* modules, since it is a module nested in the package diet. A package that has no package.d module ist still a package after all. The second link is about package modules, which is an independent thing, AFAIU.

This interpretation makes sense, taking into account that package modules were added much after the package keyword and also by the use of the word "package" in https://dlang.org/spec/module.html

RazvanN7 commented 5 years ago

I interpreted "not nested in a package" as not declaring any package modules at all.

RazvanN7 commented 5 years ago

The discussion is besides the point anyway; vibe-d imports the symbol so we might as well make it public

s-ludwig commented 5 years ago

I mean, I wouldn't mind making it public, but vibe.d doesn't use it directly (only the dietTraits property). Is using private types as a form of Voldemort types supposed to be illegal?

s-ludwig commented 5 years ago

So I've looked at the actual error now. The problem is not the package at all, it is the private that was there before version 1.4.5. The real fix is to simply upgrade the diet-ng dependency in dub to 1.4.5.

I've opened a PR: https://github.com/dlang/dub/pull/1666