Copilot-Language/copilot

`copilot-c99`: compiler warnings during tests

swt2c opened this issue · 10 comments

swt2c commented

While building copilot-c99 on Debian, we observed lots of these compiler warnings while running the tests:

main.c:9:23: warning: integer constant is so large that it is unsigned
    9 | uint64_t input_s[] = {10613134964919841005};
      |                       ^~~~~~~~~~~~~~~~~~~~

It seems that probably all that needs to be done is postfix uint64_t literals with 'u' to make the compiler aware that these are unsigned literals.

ivanperez-keera commented

The way these values are printed is controlled by the following instances:

copilot/copilot-c99/tests/Test/Copilot/Compile/C99.hs

Lines 884 to 916 in cfc565b

-- | Show a value of a given type in C.
class CShow s where
cshow :: s -> String
instance CShow Int8 where
cshow = show
instance CShow Int16 where
cshow = show
instance CShow Int32 where
cshow = show
instance CShow Int64 where
cshow = show
instance CShow Word8 where
cshow = show
instance CShow Word16 where
cshow = show
instance CShow Word32 where
cshow = show
instance CShow Word64 where
cshow = show
instance CShow Float where
cshow = show
instance CShow Double where
cshow = show

Are there any other instances apart from those for uint64_t that should be post-fixed?

swt2c commented

Are there any other instances apart from those for uint64_t that should be post-fixed?

I think it's only uint64_t as that seems to be what all the compiler warnings are referencing. If you want me to try out a change, let me know. (Sorry, my Haskell knowledge is VERY beginner.)

ivanperez-keera commented

Pinging @fdedden in case he has input to provide.

ivanperez-keera commented

Description

The code generated by copilot-c99 during tests produces warnings during compilation. The reason is that some constants included in the code are too big and the compiler is making assumptions about their necessary types. If those constants are annotated, their intention will be made clearer.

Type

  • Bug: test produce warnings during compilation.

Additional context

None.

Requester

  • Scott Talbert (Debian Developer & Debian Haskell Group)

Method to check presence of bug

Compiling copilot-c99's tests can produce warnings:

main.c:9:23: warning: integer constant is so large that it is unsigned
    9 | uint64_t input_s[] = {10613134964919841005};
      |                       ^~~~~~~~~~~~~~~~~~~~

Expected result

The above tests should compile without warnings.

Desired result

The above tests should compile without warnings.

Proposed solution

Modify the CShow instances for different types so that constants are printed with the necessary annotations.

Further notes

  • Tests are randomly generated, so there's no guarantee that code with such warnings will be produced by a specific run. This issue is therefore not always replicable unless we fix the number generator seed.
ivanperez-keera commented

Change Manager: Confirmed that the issue manifests.

ivanperez-keera commented

Technical Lead: Confirmed that the issue should be addressed.

ivanperez-keera commented

Technical Lead: Bug scheduled for fixing in Copilot 3.18.

Fix assigned to: @ivanperez-keera.

ivanperez-keera commented

Implementor: Solution implemented, review requested.

ivanperez-keera commented

Change Manager: Verified that:

  • Solution is implemented:

    • The code proposed compiles and passes all tests. Details:
      Build log: https://app.travis-ci.com/github/Copilot-Language/copilot/builds/267932537

    • The solution proposed produces the expected result. Details:
      The issue cannot be easily reproduced because it is dependent on the architecture and compiler. The correct behavior has been verified by asking the user who reported the issue to confirm that they no longer experience the problem with the solution proposed. See #479 (comment) for details.

  • Implementation is documented. Details:
    Internal change. No changes to the public documentation are needed. The new code is documented.
  • Change history is clear.
  • Commit messages are clear.
  • Changelogs are updated.
  • Examples are updated. Details:
    No updates needed.
  • Required version bumps are evaluated. Details:
    Bump not needed (internal change to tests; does not affect API).
ivanperez-keera commented

Change Manager: Implementation ready to be merged.