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

Added no-main detection #1178

Merged
merged 5 commits into from Oct 21, 2012

Conversation

Projects
None yet
6 participants
@carlor
Copy link
Contributor

carlor commented Oct 12, 2012

Reads linker error output, and gives a "no main function specified" error when proper.

@alexrp

This comment has been minimized.

Copy link
Member

alexrp commented Oct 12, 2012

This seems reasonable to me and is sorely needed - a lot of people are confused when they see a bunch of seemingly nonsensical linker errors just because they forgot main.

This does make some assumptions about the output of the linker, but I don't think that's a problem for all practical purposes. Though, is this tested on all the platforms listed in that #if?

@carlor

This comment has been minimized.

Copy link
Contributor Author

carlor commented Oct 12, 2012

@alexrp I've only tested it on my OSX machine. However, it's all POSIX, so any standards-compliant platform should work.

EDIT: now that I think about it, ld isn't POSIX. So no.

@alexrp

This comment has been minimized.

Copy link
Member

alexrp commented Oct 12, 2012

I was more curious whether the error detection works everywhere than anything else.

@carlor

This comment has been minimized.

Copy link
Contributor Author

carlor commented Oct 12, 2012

@alexrp If it doesn't detect the error, it outputs the linker error as it's always done.

@alexrp

This comment has been minimized.

Copy link
Member

alexrp commented Oct 15, 2012

Sure, and that's fine; I'm just wondering if it will actually catch the missing main error on all three POSIXes that we currently support (Linux, OS X, FreeBSD) or if this was only tested on a particular system.

@carlor

This comment has been minimized.

Copy link
Contributor Author

carlor commented Oct 15, 2012

@alexrp I tested it on Debian, there's a different error message. I'll try and make it check for that.

@carlor

This comment has been minimized.

Copy link
Contributor Author

carlor commented Oct 19, 2012

Testing on other platforms, will reopen when done.

@carlor carlor closed this Oct 19, 2012

@carlor

This comment has been minimized.

Copy link
Contributor Author

carlor commented Oct 20, 2012

Tested on Debian, modified so that error messages on other platforms can be easily added in.

@carlor carlor reopened this Oct 20, 2012

@WalterBright

This comment has been minimized.

Copy link
Member

WalterBright commented Oct 21, 2012

Clever!

WalterBright added a commit that referenced this pull request Oct 21, 2012

@WalterBright WalterBright merged commit f58578f into dlang:master Oct 21, 2012

1 check passed

default Pass: 9
Details

WalterBright added a commit that referenced this pull request Oct 21, 2012

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on 1a7014b Oct 22, 2012

The strategy of this is great, but the implementation is awkward and deviates (I think) a bit from dmd's usual style as I'll note inline.

This comment has been minimized.

Copy link
Contributor Author

carlor replied Oct 22, 2012

@andralex This is an earlier commit; I fixed some of the issues you mentioned, but not all of them. The ones I still need to work on:

  • style stuff (variable definitions, 1/0 -> true/false)
  • error handling (I miss D here)
  • fread/fwrite

I don't want to return true when nmeFound, because I still need to forward the rest of the error message. In later commits I break to the no-checking forwarding loop.

How would converting the last loop to using fread work? I'm assuming read/write a buffer. What should its size be?

Should I create another pull request which handles these three things?

This comment has been minimized.

Copy link
Member

andralex replied Oct 22, 2012

Thanks for your understanding. Sorry for misinterpreting the logic where I did. For fread/fwrite, just use a stack-allocated 64 KB buffer. Feel free to create an additional pull request - aside from the error checking issue, there's no urgency.

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

move definition to point of initialization

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

move definition to point of initialization

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

move to after the test against stream

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

must return here!

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

static const char nmeErrorMessage[] = " \"__Dmain\", referenced from:";

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

must check for errors

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

move definition to initialization

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

This fgetc/fputc approach is very slow. Should we invest in switching to fread/fwrite?

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

return true;

@andralex

This comment has been minimized.

Copy link
Member

andralex commented on src/link.c in 1a7014b Oct 22, 2012

return false;

@CyberShadow

This comment has been minimized.

Copy link
Member

CyberShadow commented Oct 22, 2012

Ooh. Can we use the linker pipe to also demangle D symbols from linker errors on the fly?

@andralex

This comment has been minimized.

Copy link
Member

andralex commented Oct 22, 2012

@CyberShadow What a great idea!!!

@MartinNowak

This comment has been minimized.

Copy link
Member

MartinNowak commented Jan 23, 2013

Calling waitpid without emptying the pipe buffer causes a deadlock for huge error messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment