Re: [Tinycc-devel] Broken commit e460f7dbb216

[Vincent Lefevre]

Hi,

On 2022-07-30 11:02:39 +0200, grischka wrote:
> On 28.07.2022 16:34, Detlef Riekenberg wrote:
> > Hi grischka.
> > 
> 
> Hi Detlef,
> 
> Please try to be more precise.  For example, in your last commit,
> you wrote:
> 
>    "Do not fail with _Alignas(0) and _Alignas(1), used by autotools"
> 
> But there wasn't anything wrong with _Alignas(1), to begin with.
> Also, the "(n > 1) &" part that you added in your one-line patch
> 
>    -  if (n <= 0 || (n & (n - 1)) != 0)
>    +  if (n < 0 || ((n > 1) & ((n & (n - 1)) != 0)))
> 
> is just redundant.  Unnecessary redundancy confuses the readers, and
> confused readers may add even more confusing stuff in sequence.
> (Now I see that "(n > 1) &" has changed to "(n > 1) &&" which, well,
> is still equally redundant.)

I changed the & to && because it is more correct and makes more sense,
but note that this is not really redundant because n is signed and
n & (n - 1) is not portable when n = 0 or 1 if two's complement isn't
necessarily used, because 0 may have several representations (well,
tcc is not designed to work on such exotic platforms, but who knows
in the future... and moreover, because of this general portability
issue, compilers may issue warnings).

> As to the "n < 0 ||" clause by the way. the C-standards seem to say:
> 
>   "Alignments are represented as values of the type size_t. Valid alignments
>    include only fundamental alignments, plus an additional implementation-
>    defined set of values, which may be empty.  Every valid alignment value
>    shall be a nonnegative integral power of two."
> 
> Well, when "size_t" means unsigned, how could it be not "nonnegative"
> then ?!?

I don't understand what you mean here. The fact is that in the code,
n is of type int, not size_t. So the case n < 0 needs to be taken
into account. Note that n comes from

                  n = expr_const();

thus may be negative.

BTW, the condition seems incomplete because large values are forbidden.
ISO C17 says in 6.2.8p2:

  A fundamental alignment is a valid alignment less than or equal to
  _Alignof (max_align_t).

and in 6.7.5p3 (a constraint):

  The constant expression shall be an integer constant expression.
  It shall evaluate to a valid fundamental alignment, or to a valid
  extended alignment supported by the implementation for an object
  of the storage duration (if any) being declared, or to zero.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)