Skip to content
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

Jump to bottom

cmd/cgo: -godefs dumps C struct members with uppercase even C struct uses lowercase #46332

Closed
Robert-M-Muench opened this issue May 23, 2021 · 7 comments
Closed

cmd/cgo: -godefs dumps C struct members with uppercase even C struct uses lowercase #46332

Robert-M-Muench opened this issue May 23, 2021 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Backlog

Comments

@Robert-M-Muench
Copy link

Robert-M-Muench commented May 23, 2021 •
edited
Loading

go version go1.16 darwin/amd64

The C struct:

struct FontMetrics {
  //! Font size.
  float size;

  union {
    struct {
      //! Font ascent (horizontal orientation).
      float ascent;
      //! Font ascent (vertical orientation).
      float vAscent;
      //! Font descent (horizontal orientation).
      float descent;
      //! Font descent (vertical orientation).
      float vDescent;
    };

    struct {
      float ascentByOrientation[2];
      float descentByOrientation[2];
    };
  };

  //! Line gap.
  float lineGap;
  //! Distance between the baseline and the mean line of lower-case letters.
  float xHeight;
  //! Maximum height of a capital letter above the baseline.
  float capHeight;

  //! Minimum x, reported by the font.
  float xMin;
  //! Minimum y, reported by the font.
  float yMin;
  //! Maximum x, reported by the font.
  float xMax;
  //! Maximum y, reported by the font.
  float yMax;

  //! Text underline position.
  float underlinePosition;
  //! Text underline thickness.
  float underlineThickness;
  //! Text strikethrough position.
  float strikethroughPosition;
  //! Text strikethrough thickness.
  float strikethroughThickness;

  // --------------------------------------------------------------------------
  #ifdef __cplusplus
  BL_INLINE void reset() noexcept { memset(this, 0, sizeof(*this)); }
  #endif
  // --------------------------------------------------------------------------
};

gets dumped as:

	fm	struct {
		Size			float32
		Anon0			[16]byte
		LineGap			float32
		XHeight			float32
		CapHeight		float32
		XMin			float32
		YMin			float32
		XMax			float32
		YMax			float32
		UnderlinePosition	float32
		UnderlineThickness	float32
		StrikethroughPosition	float32
		StrikethroughThickness	float32
	}

But can only be accessed by fm.size and not fm.Size as the output suggest. Using the output field names gives:

..\windows\text.go:168:51: fm.Anon0 undefined (type _Ctype_struct_BLFontMetrics has no field or method Anon0, but does have anon0)

The upper-case in the output is not what you need to use in your source code.

The output should match exactly what the compiler expects/does.
@icholy
Copy link

icholy commented May 23, 2021 •
edited
Loading

It's unclear what your actual problem is. You should fill out the following sections of the issue template:

### What did you expect to see?



### What did you see instead?

@mknyszek
Copy link
Contributor

@icholy I think @Robert-M-Muench is reporting that -godefs produces different output to what the cgo tool actually produces? But yeah I'm also not clear.

@Robert-M-Muench, please update your original post with more information, ideally following the issue template.

@mknyszek mknyszek added this to the Backlog milestone May 24, 2021
@mknyszek mknyszek added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label May 24, 2021
@Robert-M-Muench
Copy link
Author

I thought it was clear from the title, anyway updated my comment to make the problem more explicit.

@ianlancetaylor
Copy link
Member

The point of the -godefs option is to provide a Go definition of structs that are used by Go for cases other than import "C". There is no reason to use -godefs if you are also using import "C". And when not using cgo, we want to export the field names, so that other packages can refer to them. For example, see the golang.org/x/sys/unix package, which uses -godefs to generate many of the struct definitions.

So, what you say is true, but I don't see why it is a problem, or how it could work in any other way. Can you explain what the real issue is?

@Robert-M-Muench
Copy link
Author

There is no reason to use -godefs if you are also using import "C".

I had to use it because CGO doesn't support anonymous structs/unions. Hence, I had to find out how Go/cgo translates anonymous parts, which lead to the Anon0/anon0 discrepancy.

@ianlancetaylor
Copy link
Member

OK, but I don't see anything to change here.

By its nature, -godefs must capitalize the fields of structs that it generates. If it did not do that, it would be useless.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@mknyszek mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022
@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Dec 26, 2022
@gopherbot
Copy link
Contributor

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@github-project-automation github-project-automation bot moved this from Triage Backlog to Done in Go Compiler / Runtime Jan 26, 2023
@golang golang locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@icholy @mknyszek @Robert-M-Muench @ianlancetaylor @gopherbot @seankhliao