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.

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

struct typedef without name #270

Closed
wants to merge 28 commits into from
Closed

struct typedef without name #270

wants to merge 28 commits into from

Conversation

Konstantin8105
Copy link
Contributor

@Konstantin8105 Konstantin8105 commented Oct 23, 2017

For #265


This change is Reviewable

@elliotchance
Copy link
Owner

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


transpiler/declarations.go, line 58 at r1 (raw file):

	// Name of RecordDecl is empty
	// So, we have to save all n.Children at the base of Address
	if name == "" && !p.IsTypeAlreadyDefined(name) {

There is no need to check if the name is not defined, you are already testing that it is blank. Another way to look at it that we never want an empty name registered:

if name == "" {

Comments from Reviewable

@Konstantin8105
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


transpiler/declarations.go, line 58 at r1 (raw file):

Previously, elliotchance (Elliot Chance) wrote…

There is no need to check if the name is not defined, you are already testing that it is blank. Another way to look at it that we never want an empty name registered:

if name == "" {

Done.


Comments from Reviewable

@codecov
Copy link

codecov bot commented Oct 24, 2017

Codecov Report

Merging #270 into master will decrease coverage by 2.1%.
The diff coverage is 64.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
- Coverage   81.85%   79.74%   -2.11%     
==========================================
  Files         136      138       +2     
  Lines        5588     5663      +75     
==========================================
- Hits         4574     4516      -58     
- Misses        815      943     +128     
- Partials      199      204       +5
Impacted Files Coverage Δ
program/struct.go 66.66% <0%> (-3.93%) ⬇️
ast/position.go 80.37% <0%> (-3.47%) ⬇️
ast/ast.go 97.44% <100%> (+0.05%) ⬆️
program/program.go 70.83% <100%> (+0.62%) ⬆️
ast/disable_tail_calls_attr.go 52.94% <52.94%> (ø)
ast/alloc_size_attr.go 60% <60%> (ø)
transpiler/declarations.go 81.72% <85.71%> (-15.91%) ⬇️
darwin/ctype.go 0% <0%> (-55.11%) ⬇️
darwin/math.go 18.18% <0%> (-45.46%) ⬇️
ast/always_inline_attr.go 52.94% <0%> (-23.53%) ⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4f0b34...da60408. Read the comment docs.

@Konstantin8105
Copy link
Contributor Author

Konstantin8105 commented Oct 24, 2017

Not success only for Mac.
Now I work to fix any problem from system header entities globally.
If it interesting, so see https://github.com/Konstantin8105/c2go/tree/Preprocessor . But at the end of week, I will prepare a PR for you.

@elliotchance
Copy link
Owner

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


a discussion (no related file):
Once the build is passing this is ready to be merged.


Comments from Reviewable

@Konstantin8105
Copy link
Contributor Author

May you clarify - How I can fix the bug for Mac build if I haven't Mac? What tool can I use?

@Konstantin8105 Konstantin8105 mentioned this pull request Oct 30, 2017
@Konstantin8105
Copy link
Contributor Author

Konstantin8105 commented Oct 30, 2017

Need help:
My Mac show

 2017-10-30 06:52:52 ⌚  Konstantins-iMac in ~/work/src/github.com/elliotchance/c2go
± |structTypedef → origin ✓| → go test -tags=integration 
PASS
ok  	github.com/elliotchance/c2go	87.020s

but in travis:

    --- FAIL: TestIntegrationScripts/tests/struct.c (1.84s)
    	main_test.go:159: Expected 
    		Got: # command-line-arguments
    		build/tests/struct/main_test.go:642:8: undefined: ts_c
    		build/tests/struct/main_test.go:644:13: undefined: noarch.ByteSliceToFloat64

Don't understood - How to solve?

$ clang++ --version
Apple LLVM version 9.0.0 (clang-900.0.38)
Target: x86_64-apple-darwin17.0.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

@elliotchance
Copy link
Owner

These issues can be very painful to debug. It is likely caused by Travis having a different version of clang.

You can actually run a Travis build in debug more (this let you log into the instance and run commands): https://docs.travis-ci.com/user/running-build-in-debug-mode/

Keep in mind that clang and clang++ are part of the same package, however clang is only for C which is what c2go uses.


Reviewed 6 of 6 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@Konstantin8105
Copy link
Contributor Author

Don't found - Please check - Is that feature is switch on?
https://docs.travis-ci.com/user/running-build-in-debug-mode/ :
This feature is available for private repositories and those public repositories for which the feature is enabled.


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@elliotchance
Copy link
Owner

I see. I will try and debug it when I next get a chance.


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@elliotchance
Copy link
Owner

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


transpiler/declarations.go, line 165 at r4 (raw file):

				}
			}
			return nil, fmt.Errorf("Cannot found ast.Record")

"cannot find ast.Record"


transpiler/declarations.go, line 180 at r4 (raw file):

				}
				// removing from map, because now defined
				// delete(p.StructsEmptyName, n.Addr)

Remove or uncomment this.


transpiler/declarations.go, line 183 at r4 (raw file):

				return nil
			} else {
				p.AddMessage(p.GenerateWarningMessage(fmt.Errorf("Cannot found address for struct without name"), n))

"cannot find address for struct without name"


Comments from Reviewable

@Konstantin8105
Copy link
Contributor Author

I have to close that PR, because I do somethink wrong with github and start a new branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants