Implement nogc framework (warn about GC allocations)

[jpf91]

This pull request adds the foundation for nogc related functionality.
It implements the basics to track if a function uses the GC in exactly the same way as @safe/@trusted/@system.
On top of that it adds

• a -vgc switch which will print all implicit gc allocations, similar to -vtls
• a -nogc switch which will make all implicit GC accesses in the current module an error

It currently does not help if functions like GC.malloc or other functions which allocate are called. In order to detect this a @nogc attribute is needed. This pull requests implements the basics for @nogc including inference in templated methods, but it does not implement the @nogc attribute in the parser. I'll leave that for another pull request.

The following known allocations are intentionally not handled:

• AssertExp
• CmpExp
• AssignExp: Calls _d_arrayassign
• Hidden function errors: Are already deprecated
• SwitchErrorStatement

All these cases only allocate by throwing exceptions
and disallowing them would impact user code a lot. In
order to really solve such issues we need some way to
throw exceptions without GC-allocation.

Also not handled are indirect calls to user-defined
functions which may allocate (calling postblits in
array assignment for example). As this commit does
not provide a way to mark a function as @nogc there's
no way to know if such functions actually allocate.

These indirect calls to user defined functions must
be revisited when a @nogc attribute is introduced.

[ibuclaw]

Don't forget library calls in the backend that may do some allocation!

[jpf91]

@ibuclaw do you have an example/ hint where I should start looking?
grep RTLSYM_ backend/* showed only "harmless" library calls (although I can't say for sure, I have no clue where things like _ptrchk are defined)

EDIT: Seems I also forgot all built-in properties like .dup

[jpf91]

closed till I find some time to actually finish this.

[ghost]

Any update on this?

[ibuclaw]

I've had a look over and all cases are covered. Just needs to be cleaned up here or there.

... Oh, and rebased.

[jpf91]

@AndrejMitrovic thanks for reminding me about this pull request ;-)
@ibuclaw thanks for verifying this.

I've now moved all checks to the frontend and implemented the checks in the same way as @safe/@trusted is implemented. This means that a @nogc attribute could build on this work and that all compilers benefit immediately. I also made sure that templates defined in other modules do not show up in vgc output. (This also means that the code needs to be reviewed again to make sure we really catch all cases)
I moved the old backend-implementation here, for comparison purposes: https://github.com/jpf91/dmd/tree/vgc2

There is one important question left: What should we do for these cases:

• assert: With current druntime this allocates, but it depends on the runtime.
• CmpExp Allocates if opCMP is not overwritten(throws Exception)
• AssignExp Allocates only in error case
• DeleteExp
• Code coverage causes gc allocation
• ClassDeclaration::toObjFile hidden func error may cause gc allocation
• SwitchErrorStatement

I need some feedback here. We have these options:

• Warn in vgc, error in @nogc/-nogc code
• Warn in vgc, allow in @nogc/-nogc code
• Allow in vgc, allow in @nogc/-nogc code

[mihails-strasuns]

Regarding exceptions - druntime should really store exception instances in TLS and modify them before throwing instead of allocating new instance each time (unless I am missing some dangerous consequence)

[yebblies]

Regarding exceptions - druntime should really store exception instances in TLS and modify them before throwing instead of allocating new instance each time (unless I am missing some dangerous consequence)

Exception chaining?

[jpf91]

(Sorry, seems I need to reopen the pull request to make github refresh the commits...)

[ghost]

hidden func error

This is a deprecated feature, and currently only happens during -d as far as I know.

[jpf91]

@Dicebot some D code assumes that exceptions have 'unique' references. Exception chaining as mentioned by @yebblies is one thing, but fiber code also assume that (it stores a reference to the exception which is then rethrown in the thread context...)

Getting exceptions completely GC-free is a difficult problem. If you only care about performance it's not a big problem as you want to avoid exceptions anyway. But if you don't have a GC and still want to use exceptions I only see two solutions:

• Allocate exceptions which malloc, add a dispose method to throwable which would call free(this) and would always need to be called explicitly.
• ARC

[mihails-strasuns]

Is exception chaining expected to work with Errors too? That sounds a bit awkward.

But even for normal ones - what prevents from duplication the instance at point where it is supposed to be stored in chain? Fiber example should be OK assuming you don't yield in process of exception handling which is hardly a good idea anyway.

I am a bit worried about it because this is relatively common pattern in performance-caring D code, including one that uses fibers. If there are any issues I may need to update some code base :)

[jpf91]

Updated, now handles delete and enum aarray = ["test": 0] style array / associative array literals as well.

BTW: What's the best way to add test cases for this pull? As -nogc stops compilation I could write fail_compilation tests, but one test per file doesn't seem to be a good solution?

[ghost]

Does nogc emit any diagnostics? You can usually check multiple diagnostics in a file (although I think there's an upper limit above which dmd will stop emitting diagnostics).

grep for TEST_OUTPUT or just check one of the diag*.d files.

[ghost]

This code looks like an exact duplicate of the one above it. Why not put it in a single place before if (type)?

[ghost]

I think all of these need to be wrapped in a if (global.params.nogc) check, otherwise you may introduce too many checks in the compiler when the -nogc switch is not used. @yebblies / @9rnsr: thoughts?

[yebblies]

Array literals do not allocate if they are typed as static arrays.

[yebblies]

Is this going to error on this? It really shouldn't.

int main()
{
    return [1, 2, 3][1];
}

[9rnsr]

int main() { return [1, 2, 3][1]; }

I agree with you. And I also think following case should also be allowed even in nogc semantics.

enum n = () { auto a = [1,2,3]; return a[0] + a[1] + a[2]; }();

Indeed to realize it, we should also implement a compiler feature to skip codegen for CTFE-only functions. But it will allow to use full power of D language in the compile-time code.

[mihails-strasuns]

@yebblies

Is this going to error on this? It really shouldn't.

Right now spec does not guarantee that this won't allocate unless I have missed some change to it so it should error until such guarantees are provided.

[yebblies]

Right now spec does not guarantee [...]

The spec doesn't much of anything. Constfolding is an unavoidable part of D.

[yebblies]

This is certainly looking better. I do strongly suspect the visitor stuff can be done without inheriting from PostorderExpressionVisitor. It should be possible to do it with a single visitor class that gets passed to both the Expression and the Statement walkPostorder functions.

[jpf91]

The only reason for inheriting PostorderExpressionVisitor is this example:

int[4] arr = [1,2,3,4];

This code is handled in e2ir.c, and it simply does not visit ArrayLiteralExps in AssignExp. However I can't replicate that behavior with walkPostorder and I don't know how to check for this case in another way. Checking for

type->ty == Tarray && foreach (elem; elements)
    elem->isConst();

as Iain suggested would work, but DMD does allocate in these cases.
So if somebody knows a solution for this I could of course merge the visitors and avoid inheriting from PostorderExpressionVisitor.

[andralex]

What's the status of this? cc @jpf91

[jpf91]

Updated. Merged all visitors into one. It seems I was wrong and dmd doesn't allocate in the array literal cases that were discussed at last, so there's no need to have separate visitors.

[jpf91]

As we have vcomlumns now I could improve the messages though.
@ibuclaw quoting from:
http://forum.dlang.org/thread/ld0d79$2ife$1@digitalmars.com?page=3#post-mailman.41.1391714263.21734.digitalmars-d:40puremagic.com

it also depends on moving fprint(global.stdmsg, ...) => message(...)

Is there some pull request for this message function? Or could you post the code somewhere?

[andralex]

I wonder whether the use of GC in the in/out clauses could/should be tolerated.

[jpf91]

I guess we could allow that, as GC is allowed in asserts as well and contracts are quite similar.

(At some point we might want to have some way to complain about all GC allocations though: If we implement the betterC idea at some point we'll have to error for all GC allocations, as the runtime won't even have a GC)

[andralex]

Nice though I didn't see where transitivity was checked on a hasty pass. Also there is no test of the actual @nogc attribute.

[jpf91]

@andralex this only adds -vgc. If we really all agree that we want a @nogc attribute then I could add it (the idea is that it's simple to add it on top of these changes) but I thought introducing -vgc as a first step is less controversial and would get this merged faster. Then @nogc could be added on top of that.

(That's also the reason why there are no transitivity checks yet - they're not necessary with -vgc)

[mihails-strasuns]

I like having -vgc as a first step. It provide tool for analysis of GC usage patterns without making any language changes we can regret of.

[WalterBright]

This PR is incomplete, as for example it does not handle covariance/contravariance, but I'm pulling it as a good start.

[jpf91]

Thanks.

[dnadlinger]

Not a fan of the half-baked @nogc implementation included in the code…

[ghost]

There should be a spec pull (or well, a documentation pull) for the @gc/@nogc stuff.

[mihails-strasuns]

No, any trace of @nogc should be removed instead and only -vgc released initially.

[jpf91]

@klickverbot @Dicebot I'll open a pull request later today to remove the unnecessary @nogc parts.

@AndrejMitrovic there's no change to the spec, no attribute was introduced. There's some code which is not necessary for -vgc and I'll remove that.

[mihails-strasuns]

Thanks!

[ghost]

The commentary blew up and is hard to keep track of, but is -nogc/@nogc then a planned future feature, or what exactly is the final implementation?

[dnadlinger]

@jpf91: Thanks. It's not a big deal, but I'm afraid that if people finally decide to implement @nogc, the code would have already bit-rotted a fair bit in the meantime. And until then, it's only confusing to come across references to a feature that doesn't exist when reading through the code without having followed the discussion that led to it.

[jpf91]

@klickverbot
That's a good point and I don't mind removing that code ;-)
#3405

@AndrejMitrovic

• There's a -vgc switch now. When used it warns about most (implicit) allocations in the current module. It's in no way recursive, so even if you call GC.malloc directly it won't complain. It is useful to find hidden allocations (closures allocated on the heap, allocating array literals, array concatenation) but not for explicit allocations.
• A -nogc flag without recursive checking is kinda useless. So there are two options how -nogc could be added:
  1. If we have a @nogc attribute
  2. if we really prevent all GC allocations everywhere (think betterC, no runtime GC available). However, for the betterC case we need to know which functions are CTFE only so that a GC can still be used in these functions.
• A @nogc attribute could be based on this pull request. It's kinda important that an attribute works correctly in all cases (inference etc) so it should better be done by someone who's more familiar with these details. Both Walter and Andrei seemed to agree that @nogc is necessary, so it'll probably be implemented. (I'm not sure about that. I think pointing out hidden allocations + documentation which functions allocate could be good enough)

My long-term plans are adding some simple function reachability analysis and implement -nogc to better support systems without a GC. (Some people are using D on embedded ARM controllers and to get that stuff ready for a wider audience we need better error messages for missing GC than linker errors)

[WalterBright]

I know that this PR is incomplete. I plan to work on improving it.

[Orvid]

Ekk, the MSVC project files weren't updated with this PR to include nogc.c, which broke my local build scripts (and took me a while to find). Perhaps you could add the fix to the project files in #3405?

[dnadlinger]

@Orvid: It would be great if you could find the time submit a pull request yourself, since not everybody here uses MSVC.