chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 419 forks source link

Scoped modulus zero #6188

Open ben-albrecht opened 7 years ago

ben-albrecht commented 7 years ago

Summary of Problem

When scoping a modulus-by-zero operation, we get a backend compilation error that looks something like this:

In file included from /var/folders/gl/n_gvpvk56gq21_7_zqbhcgvh000h70/T//chpl-<user>-12648.deleteme/_main.c:42:
/var/folders/gl/n_gvpvk56gq21_7_zqbhcgvh000h70/T//chpl-<user>-12648.deleteme/checkModByZeroScope.c:18:37: warning: remainder by zero is undefined [-Wdivision-by-zero]
writeln_chpl2(((int64_t)((INT64(10) % INT64(0)))), INT64(7), INT32(51));
                                    ^ ~~~~~~~~
                                    1 warning generated.

Instead of the expected execution-time error, which you get if you declare the variables on the global scope:

checkDivByZero.chpl:7: error: halt reached - Attempt to divide by zero

The solution to this likely involves adding special-case checks for if (rhs == 0) to compiler/codegen/expr.cpp in codegenMod() and codegenOpAssign() for % and %=, respectively.

Steps to Reproduce

Source Code:

{ // <-- scope necessary to trigger code-gen evaluation and reproduce error
  var numint = 10,
      denomint = 0;

  writeln(numint % denomint);
}

Associated Future Test(s): test/compflags/checks/checkModByZeroScope.chpl #6187

Configuration Information

cassella commented 1 year ago

This works now with LLVM, producing a correct output:

checkModByZeroScope.chpl:7: error: halt reached - Attempt to compute a modulus by zero

Though it still fails the same way with CHPL_LLVM=none.

The future doesn't pass yet because it has no .bad file, and the good file expects a different error message (and has the filename wrong to boot.)

I'm not sure about even fixing the .good file because then the test will sometimes pass and sometimes fail. Maybe a suppressif? Or splitting the test to an llvm test and a non-llvm future?

cassella commented 8 months ago

I opened #24133 to split the test to one that passes under llvm and one that's a future with the gcc backend.

vasslitvinov commented 2 months ago

My preference is that this case produces a compile-time error, analogously by dividing by a "param" zero ex. writeln(x/0).

So I propose to repurpose this issue to request a good error message to be issued by our compiler, rather than having it come from the C code.