Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v6]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains seven commits:
- Merge with upstream/master
- update test
- improve handling of ignorable doc comments
suppress warning when building test code
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- call `saveDanglingDocComments` for local variable declarations
- adjust call for `saveDanglingDocComments` for enum members
- JDK-8303689: javac -Xlint could/should report on "dangling" doc comments
-
Changes: https://git.openjdk.org/jdk/pull/18527/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=05
Stats: 488 lines in 61 files changed: 389 ins; 3 del; 96 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v5]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request incrementally with one additional
commit since the last revision:
update test
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18527/files
- new: https://git.openjdk.org/jdk/pull/18527/files/f3670e7a..8ad8b818
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=03-04
Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v4]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request with a new target base due to a
merge or a rebase. The incremental webrev excludes the unrelated changes
brought in by the merge/rebase. The pull request contains five additional
commits since the last revision:
- improve handling of ignorable doc comments
suppress warning when building test code
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- call `saveDanglingDocComments` for local variable declarations
- adjust call for `saveDanglingDocComments` for enum members
- JDK-8303689: javac -Xlint could/should report on "dangling" doc comments
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18527/files
- new: https://git.openjdk.org/jdk/pull/18527/files/3f745431..f3670e7a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=03
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=02-03
Stats: 42074 lines in 1058 files changed: 18282 ins; 15937 del; 7855 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: Questions about the Hermetic Java project
Oops, I meant
__declspec(dllexport) void exportedMethod() {} with braces, not brackets
On Thu, Apr 18, 2024 at 8:27 PM Julian Waters wrote:
>
> I have good (and bad news) for you!
>
> The good news is that from memory, ld and gcc (but I assume you are
> only concerned about ld) can work entirely on cl compiled object files
> with no hitch whatsoever, and partial linking is fully functional with
> ld on Windows! Symbol hiding also works, but with a gotcha.
>
> Now, the bad news. --localize-hidden, to my knowledge, only works on
> ELF visibilities. This means that for the following file:
>
> exports.c
> __declspec(dllexport) void exportedMethod() ()
> void unexportedMethod() {}
>
> When objcopy is run over exports.o with --localize-hidden, it will
> work on the cl compiled object file just fine, but it will not
> recognise unexportedMethod() as a hidden method, because dllexport and
> hidden visibility are entirely separate concepts to gcc. The resulting
> exports-objcopy.o file will still have unexportedMethod as public.
> However, not all is lost just yet. --localize-symbol= still
> works perfectly fine, and can be used as a workaround if we can find a
> way to check for non-dllexport methods in cl object files and then
> feed that through the command line. I should note that clang also
> suffers from the same issue, as --localize-hidden on clang's objcopy
> also leaves unexportedMethod as public.
>
> Happy to help!
>
> best regards,
> Julian
>
> On Thu, Apr 18, 2024 at 6:28 PM Magnus Ihse Bursie
> wrote:
> >
> > On 2024-04-17 14:06, Julian Waters wrote:
> >
> > > Hi Magnus,
> > >
> > > Yes, I'm talking about the MSYS2 objcopy, but to a lesser extent also
> > > standalone Windows objcopy builds too. objcopy should be able to
> > > handle .o files from cl.exe (I assume that's what everyone here is
> > > after considering all the talk about .o files?), but to answer that
> > > question properly, I'd need a bit more detail. What kind of usage of
> > > objcopy do you have in mind? A general-ish command line example could
> > > be helpful
> >
> > To make a static library behave somewhat like a dynamic library, and not
> > expose all its internal symbols to the rest of the world, there are
> > basically two operations that needs to be done:
> >
> > 1) Combine all .o files into a single .o file, to make it look like it
> > was compiled by a single source code. That way, symbols that were
> > created in one source file and referenced in another will now behave as
> > if they are internal to the "compilation unit", i.e. like they were
> > declared static.
> >
> > 2) Modify the symbol status so that symbols that are not exported are
> > changed so they look like they were actually declared "static" in the
> > source code.
> >
> > These operations are based on concepts that exists in the gcc and clang
> > toolchain, about symbol visibility. I am not sure how well they
> > translate to a Microsoft setting. But, if the dll_export marker
> > corresponds to visible symbols, then I guess we should be able to
> > achieve something similar.
> >
> > What needs to be done then is:
> >
> > 1) Combine multiple .obj (COFF object files) into one.
> >
> > 2) Change the visibility of symbols that are not marked as dll_export:ed
> > to they appear like they were declared static.
> >
> > In the clang/gcc world, the first step is done by "partial linking" by
> > ld. That is our first blocker -- link.exe cannot do that. So the first
> > question is really, is there a Windows build of ld that can work on COFF
> > files to achieve this?
> >
> > The second step is done by objcopy using the "--localize-hidden"
> > argument. The second question is, could this work on a COFF object file?
> >
> > /Magnus
> >
> >
> > >
> > > best regards,
> > > Julian
> > >
> > > On Wed, Apr 17, 2024 at 5:55 PM Magnus Ihse Bursie
> > > wrote:
> > >> On 2024-04-16 07:23, Julian Waters wrote:
> > >>
> > And finally, on top of all of this, is the question of widening the
> > platform support. To support linux/gcc with objcopy is trivial, but
> > the question about Windows still remain.
> > >>> objcopy is also available on Windows, if the question about
> > >>> alternative tooling is still unanswered :)
> > >> At this point, I think support for static build on Windows is to either
> > >> require additional tooling on top of the Microsoft Visual Studio
> > >> toolchain, or to drop it completely, so I am definitely interested in
> > >> researching alternatives.
> > >>
> > >> Can objcopy (I assume this is from msys?) deal with COFF files generated
> > >> by cl?
> > >>
> > >> Switching the entire toolchain is not relevant at this point (but if a
> > >> non-Microsoft toolchain build for Windows is ever integrated, it might
> > >> get static builds with no extra work as a bonus), but I could certainly
> > >> accept the idea of having one or a few additional tools required to get
> > >> the normal Microsoft toolchain to produce static builds.
> > >>
Re: Questions about the Hermetic Java project
I have good (and bad news) for you!
The good news is that from memory, ld and gcc (but I assume you are
only concerned about ld) can work entirely on cl compiled object files
with no hitch whatsoever, and partial linking is fully functional with
ld on Windows! Symbol hiding also works, but with a gotcha.
Now, the bad news. --localize-hidden, to my knowledge, only works on
ELF visibilities. This means that for the following file:
exports.c
__declspec(dllexport) void exportedMethod() ()
void unexportedMethod() {}
When objcopy is run over exports.o with --localize-hidden, it will
work on the cl compiled object file just fine, but it will not
recognise unexportedMethod() as a hidden method, because dllexport and
hidden visibility are entirely separate concepts to gcc. The resulting
exports-objcopy.o file will still have unexportedMethod as public.
However, not all is lost just yet. --localize-symbol= still
works perfectly fine, and can be used as a workaround if we can find a
way to check for non-dllexport methods in cl object files and then
feed that through the command line. I should note that clang also
suffers from the same issue, as --localize-hidden on clang's objcopy
also leaves unexportedMethod as public.
Happy to help!
best regards,
Julian
On Thu, Apr 18, 2024 at 6:28 PM Magnus Ihse Bursie
wrote:
>
> On 2024-04-17 14:06, Julian Waters wrote:
>
> > Hi Magnus,
> >
> > Yes, I'm talking about the MSYS2 objcopy, but to a lesser extent also
> > standalone Windows objcopy builds too. objcopy should be able to
> > handle .o files from cl.exe (I assume that's what everyone here is
> > after considering all the talk about .o files?), but to answer that
> > question properly, I'd need a bit more detail. What kind of usage of
> > objcopy do you have in mind? A general-ish command line example could
> > be helpful
>
> To make a static library behave somewhat like a dynamic library, and not
> expose all its internal symbols to the rest of the world, there are
> basically two operations that needs to be done:
>
> 1) Combine all .o files into a single .o file, to make it look like it
> was compiled by a single source code. That way, symbols that were
> created in one source file and referenced in another will now behave as
> if they are internal to the "compilation unit", i.e. like they were
> declared static.
>
> 2) Modify the symbol status so that symbols that are not exported are
> changed so they look like they were actually declared "static" in the
> source code.
>
> These operations are based on concepts that exists in the gcc and clang
> toolchain, about symbol visibility. I am not sure how well they
> translate to a Microsoft setting. But, if the dll_export marker
> corresponds to visible symbols, then I guess we should be able to
> achieve something similar.
>
> What needs to be done then is:
>
> 1) Combine multiple .obj (COFF object files) into one.
>
> 2) Change the visibility of symbols that are not marked as dll_export:ed
> to they appear like they were declared static.
>
> In the clang/gcc world, the first step is done by "partial linking" by
> ld. That is our first blocker -- link.exe cannot do that. So the first
> question is really, is there a Windows build of ld that can work on COFF
> files to achieve this?
>
> The second step is done by objcopy using the "--localize-hidden"
> argument. The second question is, could this work on a COFF object file?
>
> /Magnus
>
>
> >
> > best regards,
> > Julian
> >
> > On Wed, Apr 17, 2024 at 5:55 PM Magnus Ihse Bursie
> > wrote:
> >> On 2024-04-16 07:23, Julian Waters wrote:
> >>
> And finally, on top of all of this, is the question of widening the
> platform support. To support linux/gcc with objcopy is trivial, but the
> question about Windows still remain.
> >>> objcopy is also available on Windows, if the question about
> >>> alternative tooling is still unanswered :)
> >> At this point, I think support for static build on Windows is to either
> >> require additional tooling on top of the Microsoft Visual Studio
> >> toolchain, or to drop it completely, so I am definitely interested in
> >> researching alternatives.
> >>
> >> Can objcopy (I assume this is from msys?) deal with COFF files generated
> >> by cl?
> >>
> >> Switching the entire toolchain is not relevant at this point (but if a
> >> non-Microsoft toolchain build for Windows is ever integrated, it might
> >> get static builds with no extra work as a bonus), but I could certainly
> >> accept the idea of having one or a few additional tools required to get
> >> the normal Microsoft toolchain to produce static builds.
> >>
> >> /Magnus
> >>
> >>> best regards,
> >>> Julian
> >>>
> >>> On Fri, Apr 12, 2024 at 7:52 PM Magnus Ihse Bursie
> >>> wrote:
> On 2024-04-02 21:16, Jiangli Zhou wrote:
>
> Hi Magnus,
>
> In today's zoom meeting with Alan, Ron, Liam and Chuck, we (briefly)
> discussed how to move forward contributing the static Java related
