Skip to content
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.

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

Tighten Up Augment Example #4533

Open
librasteve opened this issue Nov 19, 2024 · 4 comments
Open

Tighten Up Augment Example #4533

librasteve opened this issue Nov 19, 2024 · 4 comments
Labels
docs Documentation issue (primary issue type)

Comments

@librasteve
Copy link
Contributor

librasteve commented Nov 19, 2024

Problem

https://docs.raku.org/language/variables#The_augment_declarator

The better usage example has confused some readers:

For a better, and safer example, this is a practical way to create a class module to extend IO::Path by adding a currently missing method to yield the part of the basename left after the extension is removed. (Note there is no clear developer consensus about what to call that part or even how it should be constructed.)

unit class IO::Barename is IO::Path;
 
method new(|c) {
    return self.IO::Path::new(|c);
}
 
use MONKEY-TYPING;
augment class IO::Path {
    method barename {
        self.extension("").basename;
    }
}

Here is a recent exchange on IRC:

<botato> I'm a little confused reading the documentation for augment at https://docs.raku.org/language/variables
[03:05] 
 Raku bridge: <botato> it says "For a better, and safer example, this is a practical way to create a class module to extend IO::Path by adding a currently missing method", does "unit class IO::Barename is IO::Path;" and then "augment class IO::Path {"
[03:06] 
 Raku bridge: <botato> if i understand I think the intent was to make IO::Barename that is like IO::Path but with the barename method added
[03:07] 
 Raku bridge: <botato> but the example actually monkeypatched the barename method onto IO::Path, was that a mistake and it's supposed to be "augment class IO::Barename" or am I not understanding what's going on?
[03:41] 
 Raku bridge: <ugexe> i dont understand what it is trying to say
[03:43] 
 Raku bridge: <ugexe> i also don't understand what the example is trying to do

Suggestions

Edited in the light that there is no such thing as better and safer augment...

** I suggest that we delete the above paragraph and example entirely **

@librasteve librasteve added the docs Documentation issue (primary issue type) label Nov 19, 2024
@bo-tato
Copy link

bo-tato commented Nov 19, 2024

Thanks for improving the docs, I am still rather confused, I think the intent is to show some safer alternative to monkey patching IO::Path globally, where it only affects those that specifically opt in to it, but it is modifying IO::Path for everyone?
In the same directory I have Augment.rakumod:

unit class IO::Barename is IO::Path;

use MONKEY-TYPING;
augment class IO::Path {
    method barename {
        self.extension("").basename;
    }
}

Another.rakumod:

sub path-stuff is export {
    my $p = IO::Path.new("/etc/dhcpcd.conf");
    say $p.barename;
}

and script.raku:

use lib '.';
use Another;
use Augment;

path-stuff;

@librasteve
Copy link
Contributor Author

I edited my suggestion since IO::Barename is not needed to inherit from IO::Path if we rewrite it this way

@librasteve
Copy link
Contributor Author

librasteve commented Nov 19, 2024

@bo-tato

Yes - good check - it seems that augment is no respecter of scope - thank you for clearing that up

Since this is the case, I think that better and safer augment (as suggested by the current docs) is just a silly idea and that we may as well remove the whole concept.

I have edited the original suggested change accordingly.

EDIT: s/MONKEY-TYPING/augment/

@raiph
Copy link
Contributor

raiph commented Nov 19, 2024

Hi @librasteve

To be pedantic, use MONKEY-TYPING; does actually respect lexical scoping.

It's augment that doesn't. And it doesn't by design. Which is why there's got to be a use MONKEY-TYPING; (or use MONKEY;) in lexical scope if an augment is to be applied -- augment is a dodgy thing to do.


augment, by design, monkeys with an existing (open) class.

Note that you can't augment a role.

Why not?

Because a role is closed -- final, sealed, done and dusted -- immediately after the compiler has fully processed the role to the closing curly of its declaration.

(Per the original design there was discussion of being able to close a class too. This would have consequences, one being that one would not be able to inherit from or augment a class once it was closed. Closing classes may well be supported one day; in principle it can bring performance, correctness, ergonomic and other benefits.)


augment is designed to alter a class globally, and I don't think that's realistically going to change. But while I'm writing this comment I want to draw your attention to a related problem. (Perhaps leading you to search for related SOs, rakudo or doc issues, etc.)

Aiui the main existing problem, beyond augment's global scope (which is arguably not a problem per se because it's the whole point of augment), is related to .^compose of the augmented class.

The compiler's final step (I think) when processing a class declaration is to .^compose it. But augment does not call .^compose again. If someone is going to monkey around with types they had better understand what they're doing, including knowing precisely what calling .^compose does, and what not calling it leaves undone.

(Iirc, sub classes of an augmented class don't by default realize that their ancestor has been changed because it's .^compose that constructs one or more caches for a type. Caching is supposed to just be an optimization, but it leaks out, perhaps big time, when monkeying with types. Presumably .^compose should not only be called again as part of an augment but enhanced to propagate cache changes to its sub-classes. Or something like that. And perhaps it doesn't work; I recall it not being clear cut to me when I tried many years ago.

I'd say the time to tighten up augment as a Raku feature (I'm ignoring doc) will be post nailing down RAST. I don't have an opinion on what to do about the doc, if anything, before then.


In the meantime, there are alternatives to augment. But this comment is too long already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation issue (primary issue type)
Projects
None yet
Development

No branches or pull requests

3 participants