Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Implement nogc framework (warn about GC allocations) #1886

Merged
merged 1 commit into from

9 participants

@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
Collaborator

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.

@jpf91 jpf91 closed this
@ghost

Any update on this?

@ibuclaw
Collaborator

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
@Dicebot

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
Collaborator

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...)

@jpf91 jpf91 reopened this
@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
@Dicebot

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 Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/expression.c
((10 lines not shown))
return this;
+ }
+
+ if (elements && elements->dim != 0 && sc->func
@ghost
ghost added a note

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/expression.c
@@ -4454,6 +4474,12 @@ Expression *AssocArrayLiteralExp::semantic(Scope *sc)
return new ErrorExp();
}
+ if (keys->dim && sc->func
+ && sc->func->setGCUse (loc, "Associative array literals cause gc allocation"))
@ghost
ghost added a note

Please no whitespace before the function call.

@ghost
ghost added a note

And this applies to all code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/expression.c
@@ -5413,6 +5439,9 @@ Expression *NewExp::semantic(Scope *sc)
goto Lerr;
}
+ if (sc->func && !allocator && !onstack && sc->func->setGCUse (loc, "New causes gc allocation"))
+ error("Can not use new in @nogc code");
@ghost
ghost added a note

Enquote 'new'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/expression.c
@@ -9696,6 +9725,10 @@ Expression *DeleteExp::semantic(Scope *sc)
*/
error("cannot delete instance of COM interface %s", cd->toChars());
}
+
+ if (sc->func && sc->func->setGCUse (loc, "Delete requires gc"))
+ error("Can not use delete in @nogc code");
@ghost
ghost added a note

Ditto enquote 'delete'

@ghost
ghost added a note

There are more of these below as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/expression.c
@@ -11196,6 +11234,13 @@ Expression *AssignExp::semantic(Scope *sc)
}
else
e1 = e1->semantic(sc);
+
+ if (e1->op == TOKarraylength && sc->func
+ && sc->func->setGCUse (loc, "Setting .length causes gc allocation"))
@ghost
ghost added a note

Hmm.. but only if length is set to a larger value than its current value. Seems like this could optionally be a RT check. Still, nogc is a CT feature so we (or I) should not complicate things.

@yebblies Collaborator
yebblies added a note

Yeah, setting array length may cause gc allocation. Even if the new length is bigger, it may just expand the array into existing unused space.

@9rnsr Collaborator
9rnsr added a note

But currently, arr.capacity check depends on the getting block size of GC-heap. (See _d_arraysetcapacity in druntime code.)

On the other hand, we have an alternative safe way to shrink array length:

arr = arr[0 .. new_len];

It is a combination of slicing + binding array reference, and it's totally GC-free.
I think it is reasonable to enforce code rewriting from arr.length = n; to arr = arr[0 .. n]; under the nogc semantics.

@yebblies Collaborator
yebblies added a note

Even arr.length -= 10 ?

@9rnsr Collaborator
9rnsr added a note

Essentially yes, Because

  1. it is consistent for the cases:

    size_t u = 10;
    int n = 10;
    arr.length -= 10;   // (1)
    arr.length -= u;    // (2)
    arr.length -= n;    // (3)

    Just only allow the case (1) would be problematic in generic code.

  2. If we will add nogc infererence to the language, we cannot infer the attribute for the expression that modifying length property. Ambiguity is not good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/expression.c
@@ -11965,6 +12010,9 @@ Expression *CatAssignExp::semantic(Scope *sc)
Type *tb2 = e2->type->toBasetype();
+ if (sc->func && sc->func->setGCUse (loc, "Concatenation causes gc allocation"))
@ghost
ghost added a note

Shouldn't the type be checked here? I mean if it's an operator overload for a user-defined type it may not necessarily allocate.

@yebblies Collaborator
yebblies added a note

Operator overloads will never reach here.

@yebblies Collaborator
yebblies added a note

This one is also 'may' allocate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Unknown commented on the diff
src/func.c
@@ -2627,6 +2637,22 @@ static void MODMatchToBuffer(OutBuffer *buf, unsigned char lhsMod, unsigned char
}
/********************************************
+ * Returns true if function was declared
+ * directly or indirectly in a unittest block
+ */
+bool FuncDeclaration::inUnittest()
+{
+ Dsymbol *f = this;
@ghost
ghost added a note

this is non-null here. So you may as well make this a do/while instead of a while/do:

Dsymbol *f = this;
do
{
    if (f->isUnitTestDeclaration())
        return true;
    f = f->toParent();
} while (f);
@andralex Owner

noice pliz effect this

@andralex Owner

wait it was done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghost Show outdated diff Hide outdated diff Unknown commented on an outdated diff
src/func.c
((17 lines not shown))
+ return ((TypeFunction *)type)->gcuse == GCUSE_nogc;
+}
+
+/**************************************
+ * Mark function as using GC, warn on -vgc, return
+ * true if GC access is an error (@nogc function, -nogc)
+ */
+bool FuncDeclaration::setGCUse(Loc loc, const char* warn)
+{
+ if (setGCUse())
+ {
+ return true;
+ }
+ //Only warn about errors in 'root' modules
+ else if (global.params.vgc && getModule() && getModule()->isRoot()
+ && (!inUnittest() || global.params.nogcUnittest)) //Only warn in unittests if requested
@ghost
ghost added a note

@yebblies: I'm sure you'll have problems with the trailing // comment due to DDMD, although in this case it does document intention.

Anyway, maybe move it above the else if and rewrite it to:

/* Only warn about errors in 'root' modules.
nogcUnittest: only warn in unittests if requested */
else if (global.params.vgc && getModule() && getModule()->isRoot()
@yebblies Collaborator
yebblies added a note

This one is actually ok with the converter, because it's after the closing paren of the if. It's comments inside the condition which suck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@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?

src/expression.c
@@ -4296,7 +4296,20 @@ Expression *ArrayLiteralExp::semantic(Scope *sc)
printf("ArrayLiteralExp::semantic('%s')\n", toChars());
#endif
if (type)
+ {
+ if (elements && elements->dim != 0 && sc->func
+ && sc->func->setGCUse (loc, "Array literals cause gc allocation"))
+ {
+ error("Can not use array literals in @nogc code");
@yebblies Collaborator
yebblies added a note

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@yebblies
Collaborator

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

int main()
{
    return [1, 2, 3][1];
}
@9rnsr
Collaborator

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.

@Dicebot

@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
Collaborator

Right now spec does not guarantee [...]

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

@yebblies
Collaborator

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]; }();

That should be fine, nogc should be inferred for the lambda, and nogc checking should be disabled on the ctfe call, just like happens with this:

enum x = (a) { if (a) throw new Exception(null); return 0; }(0);

@Dicebot

The spec doesn't much of anything

If it is expected to be behavior relied upon, this MUST be added to spec before implementing some more feature in compiler relying on it. Current "it is not stated but we all know it" approach should not be encouraged.

@jpf91

I'll address all the points raised here soon, which probably means next weekend ;-)

About this case:

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

@yebblies You're of course right that this shouldn't complain. AFAICS this case cannot be detected in ArrayLiteralExp::semantic though. One solution is probably to move all GC detection code to a Visitor but I wonder whether error reporting will work as expected? The error functions would be called from the visitor then which would probably be called from FuncDeclaration::semantic and not from *Exp::semantic.Another solution could be adding a check in IndexExp::semantic which sets a flag on the ArrayLiteralExp to skip the test. Doesn't feel like a good solution though.

@yebblies
Collaborator

Right, it will have to happen after optimize has been called. Error reporting should be fine with everything moved into a visitor. IIRC blockExit+canThrow are used to determine nothrowness after semantic is done.

@jpf91 jpf91 closed this
@jpf91

Updated to use visitors. I used a visitor for the statements as well instead of adding another blockExit-like function to Statement. @yebblies is there some standard way to deal with cycles when visting Expressions?

I also addressed all issues mentioned above and added test cases. This one case:

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

does still cause an error with -nogc/-vgc: All functions including literals are considered to be @nogc if -vgc/-nogc is used. I think there's no way to avoid that. However, as yebblies pointed out with a @nogc attribute it would work as expected:

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

It might actually be nice if the frontend could track if a function is only used in CTFE. This could then be used to avoid emitting some templates if(isSomeString!T)... and it would also help here. But that's another issue.

BTW: Is there some way to get rid of duplicate messages when using -vgc? This basically happens for different template instances so I'm not sure how to avoid it.

@jpf91 jpf91 reopened this
@yebblies
Collaborator

Updated to use visitors. I used a visitor for the statements as well instead of adding another blockExit-like function to Statement. @yebblies is there some standard way to deal with cycles when visting Expressions?

Yes, StructLiteralExp is (IIRC) the only exp which can contain cycles, and it has a bunch of flags for this defined just above it in expression.h - seach for where these are used for examples.

enum n = () { auto a = [1,2,3]; return a[0] + a[1] + a[2]; }();
does still cause an error with -nogc/-vgc: All functions including literals are considered to be @nogc if -vgc/-nogc is used. I think there's no way to avoid that. However, as yebblies pointed out with a @nogc attribute it would work as expected:

This should be inferred as @gc, but it should be allowed to be called at compile time from @nogc functions. Note that all the places where purity and safety are checked have an exclusion for scopes where SCOPEctfe is set. Something similar should work here.

It might actually be nice if the frontend could track if a function is only used in CTFE. This could then be used to avoid emitting some templates if(isSomeString!T)... and it would also help here. But that's another issue.

That doesn't matter here, we only care if it is called in a CTFE context.

BTW: Is there some way to get rid of duplicate messages when using -vgc? This basically happens for different template instances so I'm not sure how to avoid it.

Not really, no.

@yebblies yebblies commented on the diff
src/func.c
@@ -2600,6 +2619,23 @@ static void MODMatchToBuffer(OutBuffer *buf, unsigned char lhsMod, unsigned char
}
/********************************************
+ * Returns true if function was declared
+ * directly or indirectly in a unittest block
+ */
+bool FuncDeclaration::inUnittest()
@yebblies Collaborator

Can't the user just mark the unittest as @gc to avoid getting errors? Less magic is better.

@jpf91
jpf91 added a note

The idea was to introduce a -nogc switch first without introducing @gc/@nogc attributes as I think it's still controversial whether we should add these. We could even remove the -nogc switch and keep only -vgc.

The problem is that without attributes these switches can't know which functions should be checked for allocations, so they check all functions. But unittests are usually not interesting in this regard and that's why this check is here.

@yebblies Collaborator

We could even remove the -nogc switch and keep only -vgc.

I think this is probably best to start with.

The problem is that without attributes these switches can't know which functions should be checked for allocations, so they check all functions. But unittests are usually not interesting in this regard and that's why this check is here.

Ok, it makes more sense in the absence of the attributes. But couldn't this be solved simply by not using both -vgc and -unittest at the same time? I guess that's not quite as good.

@jpf91
jpf91 added a note

Sorry, I didn't see this message. Unfortunately allocations in templates are only reported if the templates are instantiated. This means we usually have to use -unittest so see all allocations, but we're still not interested in the allocations in the unittests. (For example compiling std.algorithm without -unittest shows zero allocations, with unittests enabled there are quite some...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/sapply.c
((159 lines not shown))
+ }
+ void visit(DebugStatement *s)
+ {
+ if (!applyTo(s))
+ return;
+ doCond(s->statement);
+ }
+ void visit(LabelStatement *s)
+ {
+ if (!applyTo(s))
+ return;
+ doCond(s->statement);
+ }
+};
+
+void walkPreorder(Statement *s, StoppableVisitor *v)
@yebblies Collaborator

Are you sure you need to walk Preorder? What depends on the exact ordering? Also, why did you invert the return value of applyTo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/sapply.c
((137 lines not shown))
+ }
+ void visit(TryCatchStatement *s)
+ {
+ if (!applyTo(s))
+ return;
+ doCond(s->body);
+
+ for (size_t i = 0; i < s->catches->dim; i++)
+ doCond((*s->catches)[i]->handler);
+ }
+ void visit(TryFinallyStatement *s)
+ {
+ if (!applyTo(s))
+ return;
+ doCond(s->body);
+ doCond(s->finalbody);
@yebblies Collaborator

In this and many other cases (all loops?) you are not honoring the value of stop.

@jpf91
jpf91 added a note

The meaning of stop is different here. It doesn't mean stop visiting any statement, it only means stop visiting child statements. applyTo calls the user supplied visitor and if it set stop = true applyTo returns false. In that case this functions returns without visiting child statements, but visiting other statements will continue. It might not be a clever idea to reuse 'stop' for this, I'm open to better solutions.

Thinking about this I probably don't even need this for statements, but I need it for expressions.

@yebblies Collaborator

If you really need to do that (and I seriously doubt that's the best way) then you absolutely must at least name the variable something else.

More importantly, is it just me or do you never actually set stop to true?

@jpf91
jpf91 added a note

Yeah I think most of that code wasn't needed in the end. The only case is with expressions and that's a DeclExp which contains a ArrayLiteralExp.

@yebblies Collaborator

That's a good sign.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/nogc.c
((11 lines not shown))
+#include "init.h"
+#include "visitor.h"
+#include "expression.h"
+#include "statement.h"
+#include "declaration.h"
+#include "id.h"
+
+
+void walkPreorder(Statement *s, StoppableVisitor *v);
+
+/**************************************
+ * This recursively visits all expressions used in a statement
+ * and checks if they cause GC allocations. Calls func->setGCUse
+ * in case an allocation is found.
+ */
+class GCUseVisitor : public StoppableVisitor
@yebblies Collaborator

This visitor re-invents a lot of the existing postorder walker for Expression. Are you sure that can't work for you here?

@jpf91
jpf91 added a note

I totally missed that we have a PostorderExpressionVisitor. The only problem I have is that in some cases I do not want to visit all Expressions. For example if we have a ArrayLiteralExp in a DeclarationExp for a static array, visit(DeclarationExp *e) should not visit e->init. AFAICS setting stop always stops the complete visitor, but there's no way to only stop visiting the 'child expressions' of one expression.

@yebblies Collaborator

e->init is an Iniitializer, not an Expression, so it won't be automatically visited by PostorderExpressionVisitor. In any case, you could inherit from PostorderVisitor and override that one visit method if you needed to instead of reimplementing the whole thing! It's true that stop is not useful for skipping a subtree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/nogc.c
((209 lines not shown))
+ e->visiting = false;
+
+ if (e->e1 && e->e1->op == TOKvar)
+ {
+ VarExp *ve = (VarExp*)e->e1;
+ if (ve->var && ve->var->isFuncDeclaration() && ve->var->isFuncDeclaration()->ident)
+ {
+ Identifier *ident = ve->var->isFuncDeclaration()->ident;
+
+ if (strcmp(ident->toChars(), "_adSort") == 0 ||
+ strcmp(ident->toChars(), "_adSortChar") == 0 ||
+ strcmp(ident->toChars(), "_adSortWchar") == 0)
+ {
+ if (func->setGCUse(e->loc, "'sort' may cause gc allocation"))
+ {
+ e->error("Can not use 'sort' in @nogc code");
@yebblies Collaborator

Wait what? Why does sort gc allocate?

@jpf91
jpf91 added a note

The wchar/char version calls toUTF32 which GC-allocates. The memory is explicitly freed, but it's still a GC allocation (could cause a collection etc). The normal version doesn't seem to allocate, I'll remove that check.

@yebblies Collaborator

Oh yuck that's awful. So glad those are going away.

@jpf91
jpf91 added a note

Great. I've removed this check now ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jpf91

@yebblies

This should be inferred as @gc, but it should be allowed to be called at compile time from @nogc functions. Note that all the places where purity and safety are checked have an exclusion for scopes where SCOPEctfe is set. Something similar should work here.

It is inferred as @gc. But I don't even check function calls yet, that was not the intention of this pull request. Again, the problem is I don't want to introduce attributes yet, so although this code would work:

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

This wouldn't

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

and -nogc without attribute support is equal to the second example. It's not the call that causes the error, it's the fact that -nogc also enforces all functions and function literals not to allocate. If that's a problem we might remove the -nogc switch and only keep -vgc.

@jpf91 jpf91 closed this
@yebblies
Collaborator

and -nogc without attribute support is equal to the second example.

Aaah, now I get it. That's rather annoying, because it would cripple ctfe.

@jpf91

OK, next try ;-)
So what should I do about -nogc? Remove it for now?
BTW: The recent newsgroup discussion about a D subset without GC will actually have the same problem as -nogc: If there's no GC available all functions must be treated as @nogc except those which are only used in CTFE, however, we currently don't have a way to detect which functions are only used in CTFE.

@jpf91 jpf91 reopened this
@Dicebot

Until compiler can be made clever enough to detect it, some sort of @attribute(nocodegen) can help with both this issue and template bloat from constraints / template argument list algorithms.

src/nogc.c
((149 lines not shown))
+ for (size_t i = 0; i < init->value.dim; i++)
+ if (doCond(init->value[i]))
+ return;
+ }
+ void visit(ExpInitializer *init)
+ {
+ PostorderExpressionVisitor::doCond(init->exp);
+ }
+
+ // Also look if initializers might allocate
+ void visit(DeclarationExp *e)
+ {
+ VarDeclaration *var = e->declaration->isVarDeclaration();
+ if (var)
+ {
+ // Handle int[4] arr = [1,2,3,4];
@yebblies Collaborator

You should be able to just ignore all ArrayLiteralExps that are typed as static arrays, I think.

@jpf91
jpf91 added a note

You mean ArrayLiteralExp->type->ty != Tsarray ? That doesn't work for

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

Or checking the type of the VarDecl? But that wouldn't detect this

ubyte[4] result = *(new ubyte[](4LU).ptr);
@yebblies Collaborator

That doesn't work for int[4] arr = [1,2,3,4];

That's odd, it should. Oh right, it gets turned into a slice assign. Urrgh.

@ibuclaw Collaborator
ibuclaw added a note

I'm pretty certain you can just let this trickle down to the ArrayLiteralExp visitor.
ie: GDC glue checks:

elements->dim == 0;
  =>  Empty constructor - @nogc safe
type->ty == Tsarray;
  =>  Static array stack allocated - @nogc safe
type->ty == Tarray && foreach (elem; elements) elem->isConst();
  =>  Immutable array is stack allocated - @nogc safe.
default;
  =>  Calls _d_arrayliteralTX - @nogc error!
@ibuclaw Collaborator
ibuclaw added a note

At least the first two @nogc safe examples should be fine for all compiler implementations. :)

@jpf91
jpf91 added a note

DMD sometimes allocates in the third case unfortunately:

int[4] arr = [1,2,3,4]; //Doesn't allocate
int[] allocate2 = [1,2,4]; //Calls _d_arrayliteralTX

I don't know how to make this distinction in the visitor if I only have access to the ArrayLiteralExp`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@WalterBright

I suspect that an @nogc attribute is the correct approach, not a compiler switch. After all, not using the gc is a characteristic of the code, not how it is compiled. Also,

@nogc

at the beginning of the module will make everything in it nogc, and the source code itself documents it is nogc, rather than relying on the vagaries of how it is compiled.

@jpf91

@WalterBright I agree but I think the attribute should be added in a different pull request. I'll post a proof of concept that shows that it's really possible to build @nogc on top of these changes soon. I'll remove the -nogc switch, however you actually need -nogc for your 'minimal D' idea...

@yebblies If function A calls B can I simply check SCOPEctfe for the Scope which is passed to FuncDecl(A)::semantic? Or how do I access the correct Scope from the Visitor after CallExp::semantic has already run?

@jpf91 jpf91 closed this
@yebblies
Collaborator

@jpf91 Unfortunately the scope is lost after semantic. You'll need to add your own flag to your visitor and set it where ctfe is run. Since you've already run semantic, all the function calls should have been expanded already.

@yebblies
Collaborator

all the function calls

I mean all the ctfe'd function calls will have happened by now.

@jpf91

@yebblies thanks so I don't even have to check that explicitly.

I removed -nogc now and here is a proof of concept for a @nogc attribute:
https://gist.github.com/jpf91/9099430
However I think that should be added in a separate pull request and by someone more familiar with dmd internals than me.

So this is ready for another round of review, I hope it's the final one ;-)

@jpf91 jpf91 reopened this
@yebblies
Collaborator

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
Owner

What's the status of this? cc @jpf91

@jpf91 jpf91 changed the title from [WIP] implement nogc framework (warn/error about GC allocations) to Implement nogc framework (warn/error about GC allocations)
@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?

src/func.c
@@ -2077,6 +2079,23 @@ void FuncDeclaration::semantic3(Scope *sc)
f->trust = TRUSTsafe;
}
+ if (frequire)
+ checkGC(this, frequire);
@andralex Owner

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

@jpf91
jpf91 added a note

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andralex andralex commented on the diff
src/func.c
@@ -2077,6 +2079,23 @@ void FuncDeclaration::semantic3(Scope *sc)
f->trust = TRUSTsafe;
}
+ if (frequire)
+ checkGC(this, frequire);
+ if (fensure)
+ checkGC(this, fensure);
+ if (fbody)
+ checkGC(this, fbody);
+
+ if (needsClosure() && setGCUse(loc, "Using closure causes gc allocation"))
+ error("Can not use closures in @nogc code");
@andralex Owner

scope closures still work right?

@jpf91
jpf91 added a note

Yes, needsClosure is the same check as used in the backend, so this should really 100% match the backend behavior. Here's the test for that:
https://github.com/D-Programming-Language/dmd/pull/1886/files#diff-970b8289539ead3e97f260372e52a0f2R85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@andralex
Owner

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)

@jpf91 jpf91 changed the title from Implement nogc framework (warn/error about GC allocations) to Implement nogc framework (warn about GC allocations)
@jpf91 jpf91 Add -vgc switch
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.
645ba99
@Dicebot

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.

@WalterBright WalterBright merged commit 8e2b13b into from
@jpf91

Thanks.

@klickverbot
Collaborator

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.

@Dicebot

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.

@Dicebot

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?

@klickverbot
Collaborator

@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)

@jpf91 jpf91 deleted the branch
@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?

@klickverbot
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 21, 2014
  1. @jpf91

    Add -vgc switch

    jpf91 authored
    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.
This page is out of date. Refresh to see the latest.
View
5 src/declaration.h
@@ -612,6 +612,7 @@ class FuncDeclaration : public Declaration
#define FUNCFLAGpurityInprocess 1 // working on determining purity
#define FUNCFLAGsafetyInprocess 2 // working on determining safety
#define FUNCFLAGnothrowInprocess 4 // working on determining nothrow
+ #define FUNCFLAGgcuseInprocess 8 // working on determining gc usage
FuncDeclaration(Loc loc, Loc endloc, Identifier *id, StorageClass storage_class, Type *type);
Dsymbol *syntaxCopy(Dsymbol *);
@@ -631,6 +632,7 @@ class FuncDeclaration : public Declaration
bool overloadInsert(Dsymbol *s);
FuncDeclaration *overloadExactMatch(Type *t);
TemplateDeclaration *findTemplateDeclRoot();
+ bool inUnittest();
MATCH leastAsSpecialized(FuncDeclaration *g);
LabelDsymbol *searchLabel(Identifier *ident);
AggregateDeclaration *isThis();
@@ -655,6 +657,9 @@ class FuncDeclaration : public Declaration
bool isSafeBypassingInference();
bool isTrusted();
bool setUnsafe();
+ bool isNOGC();
+ bool setGCUse(Loc loc, const char *warn);
+ bool setGCUse();
bool isolateReturn();
bool parametersIntersect(Type *t);
virtual bool isNested();
View
78 src/func.c
@@ -33,6 +33,8 @@ void functionToBufferWithIdent(TypeFunction *t, OutBuffer *buf, const char *iden
void genCmain(Scope *sc);
void toBufferShort(Type *t, OutBuffer *buf, HdrGenState *hgs);
+void checkGC(FuncDeclaration *func, Statement *stmt);
+
/* A visitor to walk entire statements and provides ability to replace any sub-statements.
*/
class StatementRewriteWalker : public Visitor
@@ -2077,6 +2079,19 @@ void FuncDeclaration::semantic3(Scope *sc)
f->trust = TRUSTsafe;
}
+ if (fbody)
+ checkGC(this, fbody);
+
+ if (needsClosure() && setGCUse(loc, "Using closure causes gc allocation"))
+ error("Can not use closures in @nogc code");
@andralex Owner

scope closures still work right?

@jpf91
jpf91 added a note

Yes, needsClosure is the same check as used in the backend, so this should really 100% match the backend behavior. Here's the test for that:
https://github.com/D-Programming-Language/dmd/pull/1886/files#diff-970b8289539ead3e97f260372e52a0f2R85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ if (flags & FUNCFLAGgcuseInprocess)
+ {
+ flags &= ~FUNCFLAGgcuseInprocess;
+ if (type == f) f = (TypeFunction *)f->copy();
+ f->gcuse = GCUSEnogc;
+ }
+
// reset deco to apply inference result to mangled name
if (f != type)
f->deco = NULL;
@@ -2863,6 +2878,23 @@ static void MODMatchToBuffer(OutBuffer *buf, unsigned char lhsMod, unsigned char
}
/********************************************
+ * Returns true if function was declared
+ * directly or indirectly in a unittest block
+ */
+bool FuncDeclaration::inUnittest()
@yebblies Collaborator

Can't the user just mark the unittest as @gc to avoid getting errors? Less magic is better.

@jpf91
jpf91 added a note

The idea was to introduce a -nogc switch first without introducing @gc/@nogc attributes as I think it's still controversial whether we should add these. We could even remove the -nogc switch and keep only -vgc.

The problem is that without attributes these switches can't know which functions should be checked for allocations, so they check all functions. But unittests are usually not interesting in this regard and that's why this check is here.

@yebblies Collaborator

We could even remove the -nogc switch and keep only -vgc.

I think this is probably best to start with.

The problem is that without attributes these switches can't know which functions should be checked for allocations, so they check all functions. But unittests are usually not interesting in this regard and that's why this check is here.

Ok, it makes more sense in the absence of the attributes. But couldn't this be solved simply by not using both -vgc and -unittest at the same time? I guess that's not quite as good.

@jpf91
jpf91 added a note

Sorry, I didn't see this message. Unfortunately allocations in templates are only reported if the templates are instantiated. This means we usually have to use -unittest so see all allocations, but we're still not interested in the allocations in the unittests. (For example compiling std.algorithm without -unittest shows zero allocations, with unittests enabled there are quite some...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+{
+ Dsymbol *f = this;
@ghost
ghost added a note

this is non-null here. So you may as well make this a do/while instead of a while/do:

Dsymbol *f = this;
do
{
    if (f->isUnitTestDeclaration())
        return true;
    f = f->toParent();
} while (f);
@andralex Owner

noice pliz effect this

@andralex Owner

wait it was done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ do
+ {
+ if (f->isUnitTestDeclaration())
+ return true;
+ f = f->toParent();
+ } while (f);
+
+ return false;
+}
+
+/********************************************
* find function template root in overload list
*/
@@ -3509,6 +3541,52 @@ bool FuncDeclaration::setUnsafe()
}
/**************************************
+ * Return true if the function is marked
+ * as @nogc and therefore must not allocate.
+ */
+bool FuncDeclaration::isNOGC()
+{
+ assert(type->ty == Tfunction);
+ if (flags & FUNCFLAGgcuseInprocess)
+ setGCUse();
+ return ((TypeFunction *)type)->gcuse == GCUSEnogc;
+}
+
+/**************************************
+ * Mark function as using GC, warn on -vgc, return
+ * true if GC access is an error (@nogc)
+ */
+bool FuncDeclaration::setGCUse(Loc loc, const char* warn)
+{
+ if (setGCUse())
+ {
+ return true;
+ }
+ //Only warn about errors in 'root' modules
+ else if (global.params.vgc && getModule() && getModule()->isRoot()
+ && !inUnittest())
+ {
+ fprintf(global.stdmsg, "%s: vgc: %s\n", loc.toChars(), warn);
+ }
+ return false;
+}
+
+/**************************************
+ * Same as above, but do not warn on -vgc
+ */
+bool FuncDeclaration::setGCUse()
+{
+ if (flags & FUNCFLAGgcuseInprocess)
+ {
+ flags &= ~FUNCFLAGgcuseInprocess;
+ ((TypeFunction *)type)->gcuse = GCUSEgc;
+ }
+ else if (isNOGC())
+ return true;
+ return false;
+}
+
+/**************************************
* Returns an indirect type one step from t.
*/
View
3  src/mars.c
@@ -464,6 +464,7 @@ Usage:\n\
-version=level compile in version code >= level\n\
-version=ident compile in version code identified by ident\n\
-vtls list all variables going into thread local storage\n\
+ -vgc list all hidden gc allocations\n\
-w warnings as errors (compilation will halt)\n\
-wi warnings as messages (compilation will continue)\n\
-X generate JSON file\n\
@@ -739,6 +740,8 @@ int tryMain(size_t argc, const char *argv[])
global.params.vtls = true;
else if (strcmp(p + 1, "vcolumns") == 0)
global.params.showColumns = true;
+ else if (strcmp(p + 1, "vgc") == 0)
+ global.params.vgc = true;
else if (memcmp(p + 1, "transition", 10) == 0)
{
// Parse:
View
1  src/mars.h
@@ -95,6 +95,7 @@ struct Param
bool verbose; // verbose compile
bool showColumns; // print character (column) numbers in diagnostics
bool vtls; // identify thread local variables
+ char vgc; // identify gc usage
bool vfield; // identify non-mutable field variables
char symdebug; // insert debug symbolic information
bool alwaysframe; // always emit standard stack frame
View
4 src/mtype.c
@@ -2098,6 +2098,7 @@ Type *TypeFunction::substWildTo(unsigned)
t->isref = isref;
t->iswild = 0;
t->trust = trust;
+ t->gcuse = gcuse;
t->fargs = fargs;
return t->merge();
}
@@ -5150,6 +5151,7 @@ TypeFunction::TypeFunction(Parameters *parameters, Type *treturn, int varargs, L
this->isref = true;
this->trust = TRUSTdefault;
+ this->gcuse = GCUSEdefault;
if (stc & STCsafe)
this->trust = TRUSTsafe;
if (stc & STCsystem)
@@ -5180,6 +5182,7 @@ Type *TypeFunction::syntaxCopy()
t->isref = isref;
t->iswild = iswild;
t->trust = trust;
+ t->gcuse = gcuse;
t->fargs = fargs;
return t;
}
@@ -6161,6 +6164,7 @@ Type *TypeFunction::addStorageClass(StorageClass stc)
tf->isproperty = t->isproperty;
tf->isref = t->isref;
tf->trust = t->trust;
+ tf->gcuse = t->gcuse;
tf->iswild = t->iswild;
if (stc & STCpure)
View
8 src/mtype.h
@@ -594,6 +594,13 @@ enum TRUST
TRUSTsafe = 3, // @safe
};
+enum GCUSE
+{
+ GCUSEdefault = 0,
+ GCUSEgc = 1, // @gc (same as GCUSEdefault)
+ GCUSEnogc = 2, // @nogc
+};
+
enum PURE
{
PUREimpure = 0, // not pure at all
@@ -618,6 +625,7 @@ class TypeFunction : public TypeNext
bool isref; // true: returns a reference
LINK linkage; // calling convention
TRUST trust; // level of trust
+ GCUSE gcuse; // is gc used?
PURE purity; // PURExxxx
unsigned char iswild; // bit0: inout on params, bit1: inout on qualifier
Expressions *fargs; // function arguments
View
244 src/nogc.c
@@ -0,0 +1,244 @@
+// Compiler implementation of the D programming language
+// Copyright (c) 1999-2013 by Digital Mars
+// All Rights Reserved
+// written by Walter Bright
+// http://www.digitalmars.com
+// License for redistribution is by either the Artistic License
+// in artistic.txt, or the GNU General Public License in gnu.txt.
+// See the included readme.txt for details.
+
+#include "mars.h"
+#include "init.h"
+#include "visitor.h"
+#include "expression.h"
+#include "statement.h"
+#include "declaration.h"
+#include "id.h"
+
+bool walkPostorder(Statement *s, StoppableVisitor *v);
+bool walkPostorder(Expression *e, StoppableVisitor *v);
+
+/**************************************
+ * Look for GC-allocations
+ */
+class NOGCVisitor : public StoppableVisitor
+{
+public:
+ FuncDeclaration *func;
+
+ NOGCVisitor(FuncDeclaration *fdecl)
+ {
+ func = fdecl;
+ }
+
+ void doCond(Initializer *init)
+ {
+ if (init)
+ init->accept(this);
+ }
+
+ void doCond(Expression *exp)
+ {
+ if (exp)
+ walkPostorder(exp, this);
+ }
+
+ void visit(Initializer *init)
+ {
+ }
+ void visit(StructInitializer *init)
+ {
+ for (size_t i = 0; i < init->value.dim; i++)
+ doCond(init->value[i]);
+ }
+ void visit(ArrayInitializer *init)
+ {
+ for (size_t i = 0; i < init->value.dim; i++)
+ doCond(init->value[i]);
+ }
+ void visit(ExpInitializer *init)
+ {
+ doCond(init->exp);
+ }
+
+ void visit(Statement *s)
+ {
+ }
+ void visit(ExpStatement *s)
+ {
+ doCond(s->exp);
+ }
+ void visit(CompileStatement *s)
+ {
+ }
+ void visit(WhileStatement *s)
+ {
+ doCond(s->condition);
+ }
+ void visit(DoStatement *s)
+ {
+ doCond(s->condition);
+ }
+ void visit(ForStatement *s)
+ {
+ doCond(s->condition);
+ doCond(s->increment);
+ }
+ void visit(ForeachStatement *s)
+ {
+ doCond(s->aggr);
+ }
+ void visit(ForeachRangeStatement *s)
+ {
+ doCond(s->lwr);
+ doCond(s->upr);
+ }
+ void visit(IfStatement *s)
+ {
+ doCond(s->condition);
+ }
+ void visit(PragmaStatement *s)
+ {
+ for (size_t i = 0; i < s->args->dim; i++)
+ doCond((*s->args)[i]);
+ }
+ void visit(SwitchStatement *s)
+ {
+ doCond(s->condition);
+ }
+ void visit(CaseStatement *s)
+ {
+ doCond(s->exp);
+ }
+ void visit(CaseRangeStatement *s)
+ {
+ doCond(s->first);
+ doCond(s->last);
+ }
+ void visit(ReturnStatement *s)
+ {
+ doCond(s->exp);
+ }
+ void visit(SynchronizedStatement *s)
+ {
+ doCond(s->exp);
+ }
+ void visit(WithStatement *s)
+ {
+ doCond(s->exp);
+ }
+ void visit(ThrowStatement *s)
+ {
+ doCond(s->exp);
+ }
+
+ void visit(Expression *e)
+ {
+ }
+ void visit(DeclarationExp *e)
+ {
+ VarDeclaration *var = e->declaration->isVarDeclaration();
+ if (var)
+ {
+ doCond(var->init);
+ }
+ }
+ void visit(CallExp *e)
+ {
+ if (e->e1 && e->e1->op == TOKvar)
+ {
+ VarExp *ve = (VarExp*)e->e1;
+ if (ve->var && ve->var->isFuncDeclaration() && ve->var->isFuncDeclaration()->ident)
+ {
+ Identifier *ident = ve->var->isFuncDeclaration()->ident;
+
+ if (ident == Id::adDup)
+ {
+ if (func->setGCUse(e->loc, "'dup' causes gc allocation"))
+ {
+ e->error("Can not use 'dup' in @nogc code");
+ }
+ }
+ }
+ }
+ }
+ void visit(CatExp *e)
+ {
+ if (func->setGCUse(e->loc, "Concatenation may cause gc allocation"))
+ e->error("Can not use concatenation in @nogc code");
+ }
+ void visit(CatAssignExp *e)
+ {
+ if (func->setGCUse(e->loc, "Concatenation may cause gc allocation"))
+ e->error("Can not use concatenation in @nogc code");
+ }
+ void visit(AssignExp *e)
+ {
+ if (e->e1->op == TOKarraylength
+ && func->setGCUse(e->loc, "Setting 'length' may cause gc allocation"))
+ {
+ e->error("Can not set 'length' in @nogc code");
+ }
+ }
+ void visit(DeleteExp *e)
+ {
+ if (func->setGCUse(e->loc, "'delete' requires gc"))
+ e->error("Can not use 'delete' in @nogc code");
+ }
+ void visit(NewExp *e)
+ {
+ if (!e->allocator && !e->onstack && func->setGCUse(e->loc, "'new' causes gc allocation"))
+ e->error("Can not use 'new' in @nogc code");
+ }
+ void visit(NewAnonClassExp *e)
+ {
+ if (func->setGCUse(e->loc, "'new' causes gc allocation"))
+ e->error("Can not use 'new' in @nogc code");
+ }
+ void visit(AssocArrayLiteralExp *e)
+ {
+ if (e->keys->dim
+ && func->setGCUse(e->loc, "Associative array literals cause gc allocation"))
+ {
+ e->error("Can not use associative array literals in @nogc code");
+ }
+ }
+ void visit(ArrayLiteralExp *e)
+ {
+ bool init_const = false;
+ if (e->type->ty == Tarray)
+ {
+ init_const = true;
+ for (size_t i = 0; i < e->elements->dim; i++)
+ {
+ if (!((*e->elements)[i])->isConst())
+ {
+ init_const = false;
+ break;
+ }
+ }
+ }
+ if (e->elements && e->elements->dim != 0 && e->type->ty != Tsarray && !init_const &&
+ func->setGCUse(e->loc, "Array literals cause gc allocation"))
+ {
+ e->error("Can not use array literals in @nogc code");
+ }
+ }
+ void visit(IndexExp* e)
+ {
+ if (e->e1->type->ty == Taarray && func->setGCUse(e->loc, "Indexing an associative"
+ " array may cause gc allocation"))
+ {
+ e->error("Can not index an associative array in @nogc code");
+ }
+ }
+};
+
+void checkGC(FuncDeclaration *func, Statement *stmt)
+{
+ if (global.params.vgc)
+ {
+ NOGCVisitor gcv(func);
+ walkPostorder(stmt, &gcv);
+ }
+}
View
8 src/posix.mak
@@ -61,7 +61,7 @@ DMD_OBJS = \
class.o \
constfold.o cond.o \
declaration.o dsymbol.o \
- enum.o expression.o func.o \
+ enum.o expression.o func.o nogc.o \
id.o \
identifier.o impcnvtab.o import.o inifile.o init.o inline.o \
lexer.o link.o mangle.o mars.o module.o mtype.o \
@@ -118,7 +118,7 @@ SRC = win32.mak posix.mak osmodel.mak \
template.c lexer.c declaration.c cast.c cond.h cond.c link.c \
aggregate.h parse.c statement.c constfold.c version.h version.c \
inifile.c module.c scope.c init.h init.c attrib.h \
- attrib.c opover.c class.c mangle.c func.c inline.c \
+ attrib.c opover.c class.c mangle.c func.c nogc.c inline.c \
access.c complex_t.h \
identifier.h parse.h \
scope.h enum.h import.h mars.h module.h mtype.h dsymbol.h \
@@ -440,6 +440,9 @@ filename.o : $(ROOT)/filename.c
func.o: func.c
$(CC) -c $(CFLAGS) $<
+nogc.o: nogc.c
+ $(CC) -c $(CFLAGS) $<
+
gdag.o: $C/gdag.c
$(CC) -c $(MFLAGS) $<
@@ -726,6 +729,7 @@ gcov:
gcov enum.c
gcov expression.c
gcov func.c
+ gcov nogc.c
gcov glue.c
gcov iasm.c
gcov identifier.c
View
4 src/win32.mak
@@ -132,7 +132,7 @@ DMDMAKE=$(MAKE) -fwin32.mak C=$C TK=$(TK) ROOT=$(ROOT)
FRONTOBJ= enum.obj struct.obj dsymbol.obj import.obj id.obj \
staticassert.obj identifier.obj mtype.obj expression.obj \
optimize.obj template.obj lexer.obj declaration.obj cast.obj \
- init.obj func.obj utf.obj parse.obj statement.obj \
+ init.obj func.obj nogc.obj utf.obj parse.obj statement.obj \
constfold.obj version.obj inifile.obj cppmangle.obj \
module.obj scope.obj cond.obj inline.obj opover.obj \
entity.obj class.obj mangle.obj attrib.obj impcnvtab.obj \
@@ -179,7 +179,7 @@ SRCS= mars.c enum.c struct.c dsymbol.c import.c idgen.c impcnvgen.c utf.h \
cond.h cond.c link.c aggregate.h staticassert.h parse.c statement.c \
constfold.c version.h version.c inifile.c staticassert.c \
module.c scope.c init.h init.c attrib.h attrib.c opover.c \
- class.c mangle.c func.c inline.c access.c complex_t.h cppmangle.c \
+ class.c mangle.c func.c nogc.c inline.c access.c complex_t.h cppmangle.c \
identifier.h parse.h scope.h enum.h import.h \
mars.h module.h mtype.h dsymbol.h \
declaration.h lexer.h expression.h statement.h doc.h doc.c \
View
97 test/compilable/nogc.d
@@ -0,0 +1,97 @@
+// REQUIRED_ARGS: -vgc
+/*
+TEST_OUTPUT:
+---
+---
+*/
+
+// Tests special cases which do not allocate and shouldn't error
+// Statically typed dynamic array literals
+int[4] arr = [1,2,3,4];
+
+int testSArray()
+{
+ int[4] arr = [1,2,3,4];
+ auto x = cast(int[4])[1,2,3,4];
+ return [1, 2, 3][1];
+}
+
+enum int[] enumliteral = [1,2,3,4];
+
+void testArrayLiteral()
+{
+ void helper(int[] a){}
+ int[] allocate2 = [1,2,4];
+ int[] e = enumliteral;
+ helper([1,2,3]);
+}
+
+// String literal concatenation
+immutable s1 = "test" ~ "string";
+enum s2 = "test" ~ "string";
+
+void testLiteral()
+{
+ string s3 = "test" ~ "string";
+}
+
+//Appending to user defined type is OK
+void testAppend()
+{
+ struct App
+ {
+ ref App opOpAssign(string op)(in string s)
+ {
+ return this;
+ }
+ App opBinary(string op)(in string s) const
+ {
+ return this;
+ }
+ }
+
+ App a;
+ a ~= "test";
+ auto b = a ~ "test";
+}
+
+// mixins
+mixin(int.stringof ~ " mixInt;");
+
+// CTFE
+enum ctfe1 = "test".dup ~ "abcd";
+
+/*
+ * Can't test this with -nogc, as -nogc marks the delegate as @nogc (as it should):
+ * ------
+ * enum n = () { auto a = [1,2,3]; return a[0] + a[1] + a[2]; }();
+ * ------
+ * Correct test:
+ * ------
+ * @nogc void func()
+ * {
+ * enum n = () { auto a = [1,2,3]; return a[0] + a[1] + a[2]; }();
+ * }
+ * ------
+ */
+
+// Reading .length is OK
+void testLength()
+{
+ int[] arr;
+ auto x = arr.length;
+}
+
+// scope prevents closure allocation
+void closureHelper1(scope void delegate() d) {}
+void testClosure()
+{
+ int a;
+
+ void del1()
+ {
+ a++;
+ }
+
+ closureHelper1(&del1);
+}
View
121 test/compilable/nogc_warn.d
@@ -0,0 +1,121 @@
+/*
+REQUIRED_ARGS: -vgc
+TEST_OUTPUT:
+---
+compilable/nogc_warn.d(34): vgc: 'new' causes gc allocation
+compilable/nogc_warn.d(35): vgc: 'new' causes gc allocation
+compilable/nogc_warn.d(36): vgc: 'new' causes gc allocation
+compilable/nogc_warn.d(37): vgc: 'new' causes gc allocation
+compilable/nogc_warn.d(42): vgc: 'delete' requires gc
+compilable/nogc_warn.d(47): vgc: 'delete' requires gc
+compilable/nogc_warn.d(52): vgc: 'delete' requires gc
+compilable/nogc_warn.d(59): vgc: Associative array literals cause gc allocation
+compilable/nogc_warn.d(60): vgc: Associative array literals cause gc allocation
+compilable/nogc_warn.d(66): vgc: Setting 'length' may cause gc allocation
+compilable/nogc_warn.d(69): vgc: Setting 'length' may cause gc allocation
+compilable/nogc_warn.d(70): vgc: Setting 'length' may cause gc allocation
+compilable/nogc_warn.d(75): vgc: Concatenation may cause gc allocation
+compilable/nogc_warn.d(76): vgc: Concatenation may cause gc allocation
+compilable/nogc_warn.d(79): vgc: Concatenation may cause gc allocation
+compilable/nogc_warn.d(80): vgc: Concatenation may cause gc allocation
+compilable/nogc_warn.d(85): vgc: Using closure causes gc allocation
+compilable/nogc_warn.d(100): vgc: 'dup' causes gc allocation
+compilable/nogc_warn.d(101): vgc: 'dup' causes gc allocation
+compilable/nogc_warn.d(107): vgc: 'dup' causes gc allocation
+compilable/nogc_warn.d(108): vgc: 'dup' causes gc allocation
+compilable/nogc_warn.d-mixin-114(114): vgc: 'new' causes gc allocation
+compilable/nogc_warn.d(120): vgc: Indexing an associative array may cause gc allocation
+---
+*/
+
+struct Struct {}
+void testNew()
+{
+ auto obj = new Object();
+ auto s = new Struct();
+ auto s2 = new Struct[5];
+ auto i = new int[3];
+}
+
+void testDelete(Struct* instance)
+{
+ delete instance;
+}
+
+void testDelete(void* instance)
+{
+ delete instance;
+}
+
+void testDelete(Object instance)
+{
+ delete instance;
+}
+
+enum aaLiteral = ["test" : 0];
+void testAA()
+{
+ int[string] aa;
+ aa = ["test" : 0];
+ aa = aaLiteral;
+}
+
+void testLength()
+{
+ string s;
+ s.length = 100;
+
+ int[] arr;
+ arr.length += 1;
+ arr.length -= 1;
+}
+
+void testConcat(string sin)
+{
+ sin ~= "test";
+ string s2 = sin ~ "test";
+
+ int[] arr;
+ arr ~= 1;
+ arr ~= arr;
+}
+
+void closureHelper2(void delegate() d) {}
+
+void testClosure2()
+{
+ int a;
+
+ void del1()
+ {
+ a++;
+ }
+
+ closureHelper2(&del1);
+}
+
+void testIdup()
+{
+ int[] x;
+ auto x2 = x.idup;
+ auto s2 = "test".idup;
+}
+
+void testDup()
+{
+ int[] x;
+ auto x2 = x.dup;
+ auto s2 = "test".dup;
+}
+
+// mixins
+void testMixin()
+{
+ mixin("auto a = new " ~ int.stringof ~ " ();");
+}
+
+void testAAIndex()
+{
+ int[string] aa;
+ aa["test"] = 0;
+}
Something went wrong with that request. Please try again.