Skip to content

scala-native 0.5.9 #4510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: series/3.x
Choose a base branch
from
Open

scala-native 0.5.9 #4510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e145629
Define _POSIX_C_SOURCE for signal_helper.c
durban Oct 16, 2025
4cdbd66
SN 0.5.9
durban Oct 16, 2025
6eaad96
Move define to the beginning of the source file
durban Oct 16, 2025
58f77f5
Protect define with ifndef
durban Oct 16, 2025
e40493b
Let's try it with .generateFunctionSourcePositions(true)
durban Oct 17, 2025
fc18357
Add clue to assert
durban Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ Global / tlCommandAliases ++= Map(

lazy val nativeTestSettings = Seq(
nativeConfig ~= { c =>
c.withSourceLevelDebuggingConfig(_.enableAll.generateFunctionSourcePositions(false))
c.withSourceLevelDebuggingConfig(_.enableAll)
.withOptimize(
true
) // `false` doesn't work due to https://github.com/scala-native/scala-native/issues/4366
Expand Down
7 changes: 7 additions & 0 deletions core/native/src/main/resources/scala-native/signal_helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
#ifdef CATS_EFFECT_SIGNAL_HELPER

// we'll need POSIX for `sigaction`:
#ifndef _POSIX_C_SOURCE
Copy link
Contributor

@LeeTibbert LeeTibbert Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK _POSIX_C_SOURCE is newer than _POSIX_SOURCE. I believe that it
is supposed to have a value defined in the Open Group Standard. Something
much higher than 1.

Later: see version suggestion below and related discussion in CE Issue #4509

All the .h file code I was looking for used _POSIX_SOURCE, and not the former.
I guess the compiler will tell you/us if it can not find the symbols revealed by the
correct macro.

#define _POSIX_C_SOURCE 1

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fails (on 0.5.9) with the same "incomplete type" and "implicit declaration" errors.

Copy link
Contributor

@s5bug s5bug Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, conversation in #scala-native I realized that. Technically this should be defined before the inclusion of any other standard headers (so before stddef.h) but I doubt it matters.

It would be an issue if i.e. sigaction depended on something in stddef.h that stddef.h only defines with _POSIX_C_SOURCE active.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be defined before the inclusion of practically anything anywhere (but we can't do that, it's not up to us). So it's not bulletproof, but I don't have a better idea.

Copy link
Contributor

@s5bug s5bug Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being a .c file, the only earlier you can get is if someone passes -include to the compiler. (This is related to why you only define _POSIX_C_SOURCE in source files and not in header files: you can't be sure that your header will be included first.) TLDR I don't see why it's not bulletproof 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this should be defined before the inclusion of any other standard headers

That is standards speak for "You will regret it if you do it any other way" ;-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macro with which I am familiar is "_POSIX_SOURCE". Is "_POSIX_C_SOURCE" here an alias for the former? If not, that may explain some lack of effect.

I just made a change in SN where I added guard code
so that the definition is idempotent.
Roughly (not compiled):

#if !defined(_GNU_SOURCE)
#define _GNU_SOURCE 1
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we'll need the #if here too.

Apprently _POSIX_SOURCE is obsolete, and equivalent to _POSIX_C_SOURCE 1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeeTibbert you shouldn't need to guard it: https://godbolt.org/z/3WssGsE1r and my argument is that you shouldn't want to guard it. Getting an error when it's defined twice is good because it tells you someone is using it wrong (i.e. the only place it should be defined is at the very beginning of a source file ["translation unit"])

Copy link
Contributor

@LeeTibbert LeeTibbert Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good discussion & good points.

People, especially in large projects with build files, specify compiler options
on the command line quite frequently. The Gell-Mann concept of "If it can be done
it will be done" applies here, I think.

Guarding it or not is, I believe, one of those points where it depends upon what one
wants/intends/can_live_with. Kind of like the reason that people bet on whether
a coin flip will be heads or tails. Personal preference & keeps things interesting.

Over time I have become much more defensive when writing code which gets compiled
anywhere off my machine. I have given up the idea that I control anything, even on that/those
machine(s).

I have become both reluctant to break user code and to give up a week of my time to an
interruption when I do. The determining question here is "Can a person compiling this
code possibly supply a value which conflicts?"

That is, if the code requires a methods introduced in an Open Group version 6, what happens if they
define a version, say 1, on the command line? In that case you are correct that the conflict should
be reported.

In this case, by my possibly mistaken reading, I believe that there is no current conflicting value,
even an empty replacement or version 1. Providing the guard will prevent a complaint in the
"innocent" situation of a user specifying a value. If at all possible and if it maintains correctness
of the code, user specified values should be respected.

I believe this is more important when the code specifies a value such as 1 and a user
wants to compile with a higher Open Group version, say 8, in order to gain a V8 definition
in another file. Why annoy the user by breaking then?

For me, I am much more concerned with the case where a user can specify a conflicting definition. Say they compile their entire project with "-std=gnu17". One would then have "_GNU_SOURCE" (and/or possibly a newer macro" defined at the same time as this file defining "_POSIX_C_SOURCE".

I believe, on hasty reading, that the <signal.h> file included by signal_helper.c will messily
detect and report this situation by having multiple definitions for some entry_points/functions.

I'm off to a another lesson in humility by chasing a Scala 2 `NullPointerException` in `ConcurrentHashMap`. I fear Cats Effects is probably affected by this bug. This discussion is much more interesting that that lesson ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s5bug I this case I mostly agree with @LeeTibbert. Specifically, the scenario I'm guarding against is this: the user specifies -D_POSIX_C_SOURCE=2 (or later) when compiling; if we don't have the #ifndef, then that causes a warning. However, we're actually fine with 2 (or later). As far as I understand, for that file, all we need is that we have some _POSIX_C_SOURCE defined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The determining question here is "Can a person compiling this code possibly supply a value which conflicts?"

My view is it is more useful to tell a user "hey you shouldn't be doing this" than let it go through. If SN wants to support people doing strange things with compiler flags, that's fine by me, but for cases outside of this one that require a _POSIX_C_SOURCE other than 1, at the very least rather than

#ifndef _POSIX_C_SOURCE
#define _POSIX_C_SOURCE 123456L
#endif

could we recommend

#if defined(_POSIX_C_SOURCE)
#if _POSIX_C_SOURCE < 123456L
#error ...
#endif
#else
#define _POSIX_C_SOURCE 123456L
#endif

somewhere?

Say they compile their entire project with "-std=gnu17"

My godbolt link shows this is not an issue: a warning is only thrown when a double-define occurs (GCC's private features.h header does guard, even when -std=gnu... is set).

Specifically, the scenario I'm guarding against is this: the user specifies -D_POSIX_C_SOURCE=2 (or later) when compiling

I want to reiterate that the user should not be doing this. _POSIX_C_SOURCE should be specified on a source-file level, not broadly across a whole compilation. If SN/CE wants to support this whole-compilation case, that's fine, I just want that decision to be made with full knowledge.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to reiterate that the user should not be doing this. _POSIX_C_SOURCE should be specified on a source-file level, not broadly across a whole compilation.

I think it immediately says the developer there is POSIX inside. Something like the internal Scala Native posixlib it makes sense for that compilation unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s5bug

I want to reiterate that the user should not be doing this. _POSIX_C_SOURCE should be specified on a source-file level, not broadly across a whole compilation.

I'm not convinced this is the case; man feature_test_macros says this to me:

NOTE: In order to be effective, a feature test macro must be defined before including any header files. This can be done either in the compilation command (cc -DMACRO=value) or by defining the macro within the source code before including any headers.

It then goes on detailing various macros, including _POSIX_C_SOURCE. So it seems fine to me.

Of course it's possible that there exists some other documentation or "best practices" which says otherwise. If so, I'd be happy to read that...

#endif

#include <stddef.h>
#include <signal.h>

Expand All @@ -19,4 +25,5 @@ int cats_effect_install_handler(int signum, Handler handler) {
}
return sigaction(signum, &action, NULL);
}

#endif
2 changes: 1 addition & 1 deletion project/plugins.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ libraryDependencies += "org.scala-js" %% "scalajs-env-selenium" % "1.1.1"
addSbtPlugin("org.typelevel" % "sbt-typelevel" % "0.8.0")

addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.18.2")
addSbtPlugin("org.scala-native" % "sbt-scala-native" % "0.5.8")
addSbtPlugin("org.scala-native" % "sbt-scala-native" % "0.5.9")
addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.7")
addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.7.2")
addSbtPlugin("com.lightbend.sbt" % "sbt-java-formatter" % "0.8.0")
Expand Down
4 changes: 2 additions & 2 deletions tests/shared/src/test/scala/cats/effect/IOFiberSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class IOFiberSuite extends BaseSuite with DetectPlatform {
s <- IO(f.toString)
// _ <- IO.println(s)
_ <- f.cancel
_ <- IO(assert(s.matches(pattern)))
_ <- IO(assert(s.matches(pattern), s))
} yield ()
}

Expand All @@ -48,7 +48,7 @@ class IOFiberSuite extends BaseSuite with DetectPlatform {
_ <- IO.sleep(100.milli)
s <- IO(f.toString)
_ <- f.cancel
_ <- IO(assert(s.matches(pattern)))
_ <- IO(assert(s.matches(pattern), s))
} yield ()
}
} else {
Expand Down
Loading