http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #10 from Iain Buclaw <ibuclaw@ubuntu.com> 2012-01-30 03:55:12 PST ---
(In reply to comment #9)
> >This I think is different from how DMC++ treats the align attribute, which is
> where the conflict of interest arises.
> 
> Which means that dmd should change to match gcc for gcc platforms. The addition of an align(0) is not the right solution.

It's not an addition of align(0) - although currently I think the parser will accept any non-negative integer, which needs to be addressed.   The point Daniel raised in question is that it would be helpful if there was information available so we could tell the difference between whether the user specified align: or align(n), so we can do the right thing accordingly.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #11 from Daniel Green <venix1@gmail.com> 2012-01-30 08:06:37 PST ---
(In reply to comment #9)
> The addition of an align(0) is not the right solution.

Currently,
    `align:` becomes`align(8)`.  Ambiguous. default alignment or 8 byte?

Using 0,
    `align:` becomes `align(0)`.  Now if 0, default alignment is requested.

I'm not suggesting adding align(0).  I'm suggesting setting the internal variable to 0 when default alignment is wanted.  The reason for this is the knowledge that default alignment is required is only available to the parser and not saved.

The biggest issue with this method is that if new code is added which uses the structalign field of a scope block.  It must also check for 0.  This could be alleviated by removing direct access to structalign and adding 2 member functions.

Scope::alignsize() - return (structalign ? structalign : global.structalign);
Scope::isDefaultAlignment() - return !structalign


---

The original patch is designed to allow GDC to do what it's been doing without breaking DMD or diverging the source more than necessary.  Since you have suggested DMD should change to match gcc on gcc platforms.  It should be considered invalid as any solution for DMD will solve the problem for GDC as well.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #12 from Walter Bright <bugzilla@digitalmars.com> 2012-01-30 11:21:01 PST ---
(In reply to comment #11)
> Currently,
>     `align:` becomes`align(8)`.  Ambiguous. default alignment or 8 byte?

This is the misunderstanding.

   align:

means align to the C compiler default. IT DOES NOT MEAN align(8). How it is implemented under the hood is IRRELEVANT.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #13 from Daniel Green <venix1@gmail.com> 2012-01-30 11:40:30 PST ---
(In reply to comment #12)
> (In reply to comment #11)
> > Currently,
> >     `align:` becomes`align(8)`.  Ambiguous. default alignment or 8 byte?
> 
> This is the misunderstanding.
> 
>    align:
> 
> means align to the C compiler default. IT DOES NOT MEAN align(8). How it is implemented under the hood is IRRELEVANT.

It is RELEVANT because that information is THROWN AWAY during parsing.  ONLY the PARSER knows that C compiler default alignment is required and DOES NOT convey that information in any form.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #14 from Daniel Green <venix1@gmail.com> 2012-01-30 12:01:15 PST ---
The statement:
    `align:` becomes `align(8)`

in based entirely on how the parser handles the align attribute.

// excerpt from parse.c line 503.
case TOKalign:
{   unsigned n;

  s = NULL;
  nextToken();
  if (token.value == TOKlparen)
  {
    nextToken();
    if (token.value == TOKint32v && token.uns64value > 0)
      n = (unsigned)token.uns64value;
    else
    {   error("positive integer expected, not %s", token.toChars());
        n = 1;
    }
    nextToken();
    check(TOKrparen);
  }
  else
    n = global.structalign; // default

  a = parseBlock();
  s = new AlignDeclaration(n, a);
  break;
}

When `align:` is encountered n is set to global.structalign.  This makes the statement equal to `align(8)`.

It's not that I don't understand that `align:` means default alignment and should be treated differently than `align(8)`.  It's that only the parser knows that and currently doesn't convey that information.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #15 from Walter Bright <bugzilla@digitalmars.com> 2012-01-30 12:20:04 PST ---
Right, it's a compiler issue. Not a language issue, and no new language features or syntax are required.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #16 from Daniel Green <venix1@gmail.com> 2012-01-30 12:23:45 PST ---
(In reply to comment #15)
> Right, it's a compiler issue. Not a language issue, and no new language features or syntax are required.

Yes, I agree.  My proposal was the following.

// excerpt from parse.c line 503.
case TOKalign:
{   unsigned n;

  s = NULL;
  nextToken();
  if (token.value == TOKlparen)
  {
    nextToken();
    if (token.value == TOKint32v && token.uns64value > 0)
      n = (unsigned)token.uns64value;
    else
    {   error("positive integer expected, not %s", token.toChars());
        n = 1;
    }
    nextToken();
    check(TOKrparen);
  }
  else
-    n = global.structalign; // default
+    n = 0; // default

  a = parseBlock();
  s = new AlignDeclaration(n, a);
  break;
}

Now the compiler can test for 0 and know that default alignment is required. This removes the ambiguity with the current implementation.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396


dawg@dawgfoto.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dawg@dawgfoto.de


--- Comment #17 from dawg@dawgfoto.de 2012-01-30 17:58:26 PST ---
https://github.com/D-Programming-Language/dmd/pull/657

The pull request additionally solves the broken header generation
for default align. Also note that adding this won't change the meaning of
align(0), as 0 is already an error.

PS:
We should definitely check at some point that alignment is a power of two.
There is already code relying on this (AggregateDeclaration::alignmember).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #18 from Iain Buclaw <ibuclaw@ubuntu.com> 2012-01-31 00:05:34 PST ---
(In reply to comment #17)
> PS:
> We should definitely check at some point that alignment is a power of two.
> There is already code relying on this (AggregateDeclaration::alignmember).

I think it would also make sense to disallow any align(n) value greater than
align(16) for 32bit, and possibly align(32) for 64bit platforms.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=7396



--- Comment #19 from Walter Bright <bugzilla@digitalmars.com> 2012-01-31 01:17:43 PST ---
I do too.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------