`#pragma GCC unroll 0` not allowed: `invalid value '0'; must be positive`

[MonsterDruide1]

#pragma GCC unroll 0 does not compile, as apparently the specified number needs to be positive.

/app/main.cpp:4:24: error: invalid value '0'; must be positive
    4 |     #pragma GCC unroll 0
      |                        ^
1 error generated.

However, from https://gcc.gnu.org/onlinedocs/gcc/Loop-Specific-Pragmas.html:

The values of 0 and 1 block any unrolling of the loop.

The program also successfully compiles with GCC, only clang seems to have this implemented incorrectly.

Godbolt: https://godbolt.org/z/bEPo64GsP

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (MonsterDruide1)

`#pragma GCC unroll 0` does not compile, as apparently the specified number needs to be positive.

/app/main.cpp:4:24: error: invalid value '0'; must be positive
    4 |     #pragma GCC unroll 0
      |                        ^
1 error generated.

However, from <https://gcc.gnu.org/onlinedocs/gcc/Loop-Specific-Pragmas.html>:
> The values of 0 and 1 block any unrolling of the loop.

The program also successfully compiles with GCC, only clang seems to have this implemented incorrectly.

Godbolt: https://godbolt.org/z/bEPo64GsP

[MonsterDruide1]

Additional info from discussion on Discord:

• There is a test to check for the error on 0:

  llvm-project/clang/test/Parser/pragma-unroll.cpp

  Lines 43 to 44 in 2685256

  	/* expected-error {{invalid value '0'; must be positive}} */ #pragma unroll(0)
  	/* expected-error {{invalid value '0'; must be positive}} */ #pragma unroll 0

• The check is implemented here:

  llvm-project/clang/lib/Sema/SemaExpr.cpp

  Line 3903 in 2685256

  bool ValueIsPositive = ValueAPS.isStrictlyPositive();

• This behaviour does not seem to have changed over time, as at the time of the first release with this pragma (8.1, see changelog) the documentation already allows 0 as value: https://web.archive.org/web/20180331074143/http://gcc.gnu.org/onlinedocs/gcc/Loop-Specific-Pragmas.html

[shafik]

If we look at c724a83 where this change was implemented it looks like the behavior did not change.

[yronglin]

I've a PR to fix this issue #88666 .

[MonsterDruide1]

Great, thanks for handling it @yronglin !