Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue 9047 - Expression requiring std.math fails with local import #1438

Closed
wants to merge 1 commit into from

5 participants

@yebblies
Collaborator

I really hate this part of the compiler!

@ibuclaw
Collaborator

You think you've got problems, I put in a further check to say "hey! looks like you haven't imported std.math, well that's no problem as I can handle it in the back end."

As opposed to the error emitted by dmd. :)

@yebblies
Collaborator

I think it's time to look at moving std.math.pow to druntime...

@klickverbot
Collaborator

Yes. Let's not pollute Scope with this nonsense.

@yebblies
Collaborator

Where should pow go in druntime?

@AndrejMitrovic
Collaborator

If we move it to druntime won't the code still need an import to the module where pow is defined?

@yebblies
Collaborator

No. Many language features (eg ~ new throw) are lowered to druntime calls without requiring imports. It doesn't make any sense for a language feature to require an import to make it work - or for the compiler to depend on a supposedly optional standard library.

It may be worth merging this in the meantime though...

@AndrejMitrovic
Collaborator

It may be worth merging this in the meantime though...

I'd rather we go ahead with putting it in druntime. I don't like this hack at all.

@braddr
Owner

This need's don's input. In the past, he's been against putting it in druntime.

@ibuclaw
Collaborator

No. Many language features (eg ~ new throw) are lowered to druntime calls without requiring imports. It doesn't make any sense for a language feature to require an import to make it work - or for the compiler to depend on a supposedly optional standard library.

Actually, pow() is rather special in that it does require to be imported, as the function called is CTFE-able. There are several tests that depend on this.

@AndrejMitrovic AndrejMitrovic Fixes Issue 9047 - Expression requiring std.math fails with local import
Mark scoped symbol where std.math is imported to allow the
creation of a proper lookup expression to pow() function.
b581ce9
@AndrejMitrovic
Collaborator

Well, it's been a year. Any fresh opinions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 10, 2013
  1. @AndrejMitrovic

    Fixes Issue 9047 - Expression requiring std.math fails with local import

    AndrejMitrovic authored
    Mark scoped symbol where std.math is imported to allow the
    creation of a proper lookup expression to pow() function.
This page is out of date. Refresh to see the latest.
View
2  src/dsymbol.c
@@ -820,6 +820,7 @@ ScopeDsymbol::ScopeDsymbol()
symtab = NULL;
imports = NULL;
prots = NULL;
+ hasStdMathImport = false;
}
ScopeDsymbol::ScopeDsymbol(Identifier *id)
@@ -829,6 +830,7 @@ ScopeDsymbol::ScopeDsymbol(Identifier *id)
symtab = NULL;
imports = NULL;
prots = NULL;
+ hasStdMathImport = false;
}
Dsymbol *ScopeDsymbol::syntaxCopy(Dsymbol *s)
View
1  src/dsymbol.h
@@ -267,6 +267,7 @@ class ScopeDsymbol : public Dsymbol
Dsymbols *imports; // imported Dsymbol's
unsigned char *prots; // array of PROT, one for each import
+ bool hasStdMathImport; // std.math import (if 'this' is a function => local import)
ScopeDsymbol();
ScopeDsymbol(Identifier *id);
View
42 src/expression.c
@@ -12128,38 +12128,44 @@ Expression *PowExp::semantic(Scope *sc)
return e;
}
- static int importMathChecked = 0;
+ static bool importMathChecked = false;
static bool importMath = false;
+ static bool isLocalImport = false;
if (!importMathChecked)
{
- importMathChecked = 1;
- for (size_t i = 0; i < Module::amodules.dim; i++)
- { Module *mi = Module::amodules[i];
- //printf("\t[%d] %s\n", i, mi->toChars());
- if (mi->ident == Id::math &&
- mi->parent->ident == Id::std &&
- !mi->parent->parent)
+ importMathChecked = true;
+ if (sc && sc->module && sc->module->hasStdMathImport)
+ importMath = true;
+
+ for (Scope *s = sc;
+ s && s->scopesym && !s->scopesym->isPackage() && !s->scopesym->isModule();
+ s = s->enclosing)
+ {
+ if (s->scopesym->hasStdMathImport)
{
importMath = true;
- goto L1;
+ isLocalImport = true;
+ break;
}
}
+ }
+
+ if (!importMath)
+ {
error("must import std.math to use ^^ operator");
return new ErrorExp();
+ }
- L1: ;
+ if (isLocalImport)
+ { // use 'std.math' for local import
+ e = new IdentifierExp(loc, Id::std);
}
else
- {
- if (!importMath)
- {
- error("must import std.math to use ^^ operator");
- return new ErrorExp();
- }
+ { // use '.std.math' for regular import
+ e = new IdentifierExp(loc, Id::empty);
+ e = new DotIdExp(loc, e, Id::std);
}
- e = new IdentifierExp(loc, Id::empty);
- e = new DotIdExp(loc, e, Id::std);
e = new DotIdExp(loc, e, Id::math);
if (e2->op == TOKfloat64 && e2->toReal() == 0.5)
{ // Replace e1 ^^ 0.5 with .std.math.sqrt(x)
View
7 src/import.c
@@ -242,6 +242,13 @@ void Import::semantic(Scope *sc)
mod->semantic();
+ if (sc && sc->scopesym && mod->ident == Id::math
+ && mod->parent->ident == Id::std && !mod->parent->parent)
+ {
+ sc->scopesym->hasStdMathImport = true;
+ sc->module->hasStdMathImport = true;
+ }
+
if (mod->needmoduleinfo)
{ //printf("module4 %s because of %s\n", sc->module->toChars(), mod->toChars());
sc->module->needmoduleinfo = 1;
View
21 test/compilable/test9047a.d
@@ -0,0 +1,21 @@
+// PERMUTE_ARGS:
+
+// sentinel: should not be picked up
+struct std { struct math { @disable static void pow(T...)(T t) { } } }
+
+void t1()
+{
+ import std.math;
+ auto f = (double a, double b) => a ^^ b;
+}
+
+void t2()
+{
+ import std.math;
+ {
+ auto f = (double a, double b) => a ^^ b;
+ {
+ auto y = (double a, double b) => a ^^ b;
+ }
+ }
+}
View
22 test/compilable/test9047b.d
@@ -0,0 +1,22 @@
+// PERMUTE_ARGS:
+
+import std.math;
+
+void t1()
+{
+ // sentinel: should not be picked up
+ struct std { struct math { @disable static void pow(T...)(T t) { } } }
+ auto f = (double a, double b) => a ^^ b;
+}
+
+void t2()
+{
+ // sentinel: should not be picked up
+ struct std { struct math { @disable static void pow(T...)(T t) { } } }
+ {
+ auto f = (double a, double b) => a ^^ b;
+ {
+ auto y = (double a, double b) => a ^^ b;
+ }
+ }
+}
View
9 test/compilable/test9047c.d
@@ -0,0 +1,9 @@
+// PERMUTE_ARGS:
+
+import std.math;
+
+void t1()
+{
+ import std.math;
+ auto f = (double a, double b) => a ^^ b;
+}
View
40 test/fail_compilation/diag9047.d
@@ -0,0 +1,40 @@
+/*
+TEST_OUTPUT:
+---
+fail_compilation/diag9047.d(17): Error: must import std.math to use ^^ operator
+fail_compilation/diag9047.d(22): Error: must import std.math to use ^^ operator
+fail_compilation/diag9047.d(28): Error: must import std.math to use ^^ operator
+fail_compilation/diag9047.d(32): Error: must import std.math to use ^^ operator
+fail_compilation/diag9047.d(39): Error: must import std.math to use ^^ operator
+---
+*/
+
+// sentinel: should not be picked up
+struct std { struct math { @disable static void pow(T...)(T t) { } } }
+
+void f1()
+{
+ auto f = (double a, double b) => a ^^ b;
+}
+
+void f2()
+{
+ auto f2 = (double a, double b) => a ^^ b;
+ import std.math;
+}
+
+void f3()
+{
+ auto f1 = (double a, double b) => a ^^ b;
+ {
+ import std.math;
+ }
+ auto f2 = (double a, double b) => a ^^ b;
+}
+
+void f4()
+{
+ // sentinel: should not be picked up
+ struct std { struct math { @disable static void pow(T...)(T t) { } } }
+ auto f = (double a, double b) => a ^^ b;
+}
Something went wrong with that request. Please try again.