www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - FYI: Be careful with imports when using public:

reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
I just thought that I'd point this out, since I got bit by it, and others
might be in the same boat, but apparently, if you have code such as

    struct Foo
    {
    public:
        import std.range.primitives;
    ...
    }

then the import is treated as public (and the same goes if you use public:
at the module level and have module-level imports under it). In this
particular case, the confusing result is that trying to call functions in
std.range.primitives (such as walkLength or moveFront) on an instance of Foo
using UFCS will result in the code not compiling (whereas using the normal
function call syntax works just fine), and the error messages don't make it
all clear as to why, so it took me quite a while to figure out what the
problem was.

In retrospect, it seems like it probably should have been obvious that using
public: would affect all imports below it in that scope, just like it does
with any of the symbols in that scope, but since I rarely do anything with
public imports, I tend to forget that they're a thing, and it had never
occurred to me that public: could affect imports (or if it had, I completely
forgot).

So, for those of you who use public: and private: (rather than marking each
symbol individually with public or private) should be careful to make sure
that no imports are under a public: unless that's what you actually want
(which you probably don't, since that's rarely desirable; usually, that sort
of thing would only be done in a package.d module).

I don't know if the issue that I had with UFCS is expected (probably), but
the larger issue is of course that you don't want to accidentally make
imports public - especially in libraries - so clearly, anyone using public:
needs to be careful with their imports (or just mark symbols with public or
private individually instead, which has its own pros and cons, but it won't
result in accidentally making imports public, so that would definitely be
one of its pros).

- Jonathan M Davis
Oct 10 2023
next sibling parent reply Salih Dincer <salihdb hotmail.com> writes:
On Tuesday, 10 October 2023 at 07:43:26 UTC, Jonathan M Davis 
wrote:

 ... In this particular case, the confusing result is that 
 trying to call functions in std.range.primitives (such as 
 walkLength or moveFront) on an instance of Foo using UFCS will 
 result in the code not compiling (whereas using the normal 
 function call syntax works just fine), and the error messages 
 don't make it all clear as to why, so it took me quite a while 
 to figure out what the problem was.


I didn't experience any compilation errors or import problems 
either. Moreover, I also tried selective import. For example:


```d

--main.d--
import publicCase;

import std.stdio;
void main() {
   auto s = S(" No Problem ");
   s.wordCount.writeln(": ", s);
   // 2: S(" No Problem ")
}

--publicCase.d--
module publicCase;

struct S
{
   private:
     string str;

   public:
    import std.range.primitives;// : walkLength;
    import std.algorithm;// : splitter;

   size_t length() => str.length;
   size_t wordCount() => str.splitter.walkLength;
}
```

SDB 79
Oct 10 2023
parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Tuesday, October 10, 2023 9:53:07 PM MDT Salih Dincer via Digitalmars-d 
wrote:

 On Tuesday, 10 October 2023 at 07:43:26 UTC, Jonathan M Davis

 wrote:

 ... In this particular case, the confusing result is that
 trying to call functions in std.range.primitives (such as
 walkLength or moveFront) on an instance of Foo using UFCS will
 result in the code not compiling (whereas using the normal
 function call syntax works just fine), and the error messages
 don't make it all clear as to why, so it took me quite a while
 to figure out what the problem was.


 I didn't experience any compilation errors or import problems
 either. Moreover, I also tried selective import. For example:


 ```d

 --main.d--
 import publicCase;

 import std.stdio;
 void main() {
    auto s = S(" No Problem ");
    s.wordCount.writeln(": ", s);
    // 2: S(" No Problem ")
 }

 --publicCase.d--
 module publicCase;

 struct S
 {
    private:
      string str;

    public:
     import std.range.primitives;// : walkLength;
     import std.algorithm;// : splitter;

    size_t length() => str.length;
    size_t wordCount() => str.splitter.walkLength;
 }
 ```

 SDB 79


The problem is not using the imports inside of the struct where you have the
public import (since whether imports are public or not has no impact on the
part of the code where they're imported, just on other modules importing
that code). The problem is using them on the struct from within another
module. So, if you have something like

a.d
---
struct Range
{
public:
    import std.range.primitives;

    int front() { return _i; }
    void popFront() { ++_i;}
    bool empty() { return _i == 10; }

private:
    int _i;
}

main.d
------
void main()
{
    import std.range.primitives;
    import a;
    auto l = Range.init.walkLength();
}

This will result in

main.d(5): Error: none of the overloads of template 
`std.range.primitives.walkLength` are callable using argument types `!()()`
/usr/local/include/dmd/std/range/primitives.d(1767):        Candidates are: 
`walkLength(Range)(Range range)`
/usr/local/include/dmd/std/range/primitives.d(1795):                        
`walkLength(Range)(Range range, const size_t upTo)`

- Jonathan M Davis
Oct 10 2023
parent reply Salih Dincer <salihdb hotmail.com> writes:
On Wednesday, 11 October 2023 at 05:31:33 UTC, Jonathan M Davis 
wrote:

 The problem is not using the imports inside of the struct where 
 you have the public import (since whether imports are public or 
 not has no impact on the part of the code where they're 
 imported, just on other modules importing that code). The 
 problem is using them on the struct from within another module.


I see! In this situation there are 2+1 (the first one doesn't 
count) solutions if I do not remember wrong:

The first thing is not to use "public:" but still be careful. 
Because the imports may accidentally be in scope, as you 
mentioned.

Other is "Renamed Imports", a local name for an import can be 
given. For example:

```d
   public:

      import std.range.primitives : wl = walkLength;
```

Finally, use selective import again and define it inside the 
member function. For example:

```d
   public:

     size_t length() {
       import std.range.primitives : walkLength;
       return this.walkLength;
     }
```

Thank you very much for this information. There is a huge 
difference between "import" alone and "public import"!

SDB 79
Oct 11 2023
next sibling parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Wednesday, October 11, 2023 5:34:50 AM MDT Salih Dincer via Digitalmars-d 
wrote:

 On Wednesday, 11 October 2023 at 05:31:33 UTC, Jonathan M Davis

 wrote:

 The problem is not using the imports inside of the struct where
 you have the public import (since whether imports are public or
 not has no impact on the part of the code where they're
 imported, just on other modules importing that code). The
 problem is using them on the struct from within another module.


 I see! In this situation there are 2+1 (the first one doesn't
 count) solutions if I do not remember wrong:

 The first thing is not to use "public:" but still be careful.
 Because the imports may accidentally be in scope, as you
 mentioned.

 Other is "Renamed Imports", a local name for an import can be
 given. For example:

 ```d
    public:

       import std.range.primitives : wl = walkLength;
 ```


I wouldn't advise that, since I'm pretty sure that that results in having wl
as a public member of the type that it's in (or of the module that it's in
if you do it at at the module-level), and you presumably don't want to be
creating any extra public symbols like that. In general, simply making sure
that the import is before public: solves the issue without having to jump
through any hoops - or using public as an attribute rather than as a label
also solves the problem.


 Finally, use selective import again and define it inside the
 member function. For example:

 ```d
    public:

      size_t length() {
        import std.range.primitives : walkLength;
        return this.walkLength;
      }
 ```


In general, it's advised to scope imports as tightly as possible to avoid
what they impact (as well as to make it clearer where they're used so that
you know where symbols come from and when you can remove the imports when
refactoring), but you do sometimes need them outside of functions (e.g. for
the return types), so while local imports help, they don't always solve the
problem.

Personally, I ran into this issue, because I just wanted to have the
necessary import to get the range primitives for arrays without having to
worry about where within the member functions I was using them, and it would
have been fine if I'd put the import before the public:, but I didn't, so I
ran into issues.


 Thank you very much for this information. There is a huge
 difference between "import" alone and "public import"!

 SDB 79


Ultimately, what it comes down to is that if you use public:, you need to be
careful where you place imports in relation to it. You don't want to be
creating public imports by accident (and almost never want to create them on
purpose; package.d is the primary counter-example). Of course, you can
choose to just avoid public: to avoid the problem (in which case, presumably
you just use public as an attribute directly on the symbols), but the point
is that if you do use public:, you need to be careful where you put it. And
it's easy to forget that it impacts imports, since most of us don't do much
with public imports.

- Jonathan M Davis
Oct 11 2023
prev sibling parent user1234 <user1234 12.de> writes:
On Wednesday, 11 October 2023 at 11:34:50 UTC, Salih Dincer wrote:

 Other is "Renamed Imports", a local name for an import can be 
 given. For example:

 ```d
   public:

      import std.range.primitives : wl = walkLength;
 ```


other is to use a `from` instance alias and to fully qualify your 
accesses:

```d
module m;

alias from_prims = from!"std.range.primitives";

void v(T)(ref T t)
{
      const wl = from_prims.walkLength(t);
      ...
}
```

that way you can just ignore whether the alias is public or not.
(note: yes I know that this also implies that UFCS has to be 
sacrificed)
Oct 11 2023
prev sibling parent mw <m g.c> writes:
On Tuesday, 10 October 2023 at 07:43:26 UTC, Jonathan M Davis 
wrote:

 I just thought that I'd point this out, since I got bit by it, 
 and others might be in the same boat, but apparently, if you 
 have code such as

     struct Foo
     {
     public:
         import std.range.primitives;
     ...
     }



I always try to put all my imports at the top of the source 
files, as long as there is no conflicts -- the same practice as I 
do in Python.

For the reason to avoid any unexpected consequences, e.g. as you 
described.
Oct 10 2023