convert mscoffobj.c to D by WalterBright · Pull Request #8876 · dlang/dmd

WalterBright · 2018-10-27T07:33:07Z

No description provided.

dlang-bot · 2018-10-27T07:33:09Z

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8876"

12345swordy · 2018-10-27T22:24:43Z

Auto Tester have failed for the Win_32_64 platform. "object.Error@(0): Access Violation".

rainers · 2018-10-28T08:15:23Z

src/dmd/backend/mscoffobj.d

 * Input:
 *      seg =           segment index that symbol is defined in
- *      s ->            symbol
+ *      s .            symbol


I guess DAutoTest doesn't like this one.

rainers · 2018-10-28T08:15:45Z

src/dmd/backend/mscoffobj.d

 * Output a common block definition.
 * Input:
- *      p ->    external identifier
+ *      p .    external identifier


rainers · 2018-10-28T08:16:13Z

src/dmd/backend/mscoffobj.d

 *      seg =   where the address is going (CODE or DATA)
 *      offset =        offset within seg
- *      s ->            Symbol table entry for identifier
+ *      s .            Symbol table entry for identifier


WalterBright · 2018-10-28T09:15:03Z

@rainers thanks for finding the problem. I don't know how you found it in the log, all I could find was:

The dependency resolution process is taking too long. The dependency graph is likely hitting a pathological case in the resolution algorithm. Please file a bug report at https://github.com/dlang/dub/issues and mention the package recipe that reproduces this error.
posix.mak:766: recipe for target 'dpl-docs' failed

so I assumed it was some environmental problem.

rainers · 2018-10-28T09:28:10Z

@rainers thanks for finding the problem. I don't know how you found it in the log

I just noticed during review, I think there will be more (IIRC some parameter names in the doc didn't match the function signature, but couldn't find them now). DAutoTest seems to fail for other reasons on all PRs, see e.g. #8878 (comment)

rainers · 2018-10-28T09:33:38Z

src/win32.mak


-$G/mscoffobj.obj : $C\mscoff.h $C\mscoffobj.c
-	$(CC) -c -o$@ $(MFLAGS) -I$D;$(ROOT) -I$G $C\mscoffobj
+$G/mscoffobj.obj : $C\mscoff.d $C\mscoffobj.d


This compiles the 2 files into different object files, but not both are passed to the linker. Simplest to compile them seperately.

One wonders why that suddenly stopped working (and it works on my machine). Sigh.

thewilsonator

I know you try to not make unnecessary changes when converting files but it's probably a good idea to address this even if you just comment the test and one of the branches.

thewilsonator · 2018-10-28T12:23:58Z

src/dmd/backend/mscoffobj.d

-                                rel.r_vaddr = r->offset;
-                                rel.r_symndx = s->Sxtrnnum;
+                                rel.r_vaddr = cast(uint)r.offset;
+                                rel.r_symndx = s.Sxtrnnum;


DScanner complains that this and the above branch are identical. This is the cause of the buildkite failure.

I commented them out.

thewilsonator · 2018-10-28T12:24:39Z

src/dmd/backend/mscoffobj.d

-                                s->Sclass == SCcomdef ||
-                                s->Sclass == SCcomdat ||
-                                s->Sclass == SCglobal)
+                            if (s.Sclass == SCextern ||


wilzbach · 2018-10-29T12:52:24Z

Rebased to restart DAutoTest

andralex

onward!

WalterBright requested review from MartinNowak and wilzbach as code owners October 27, 2018 07:33

WalterBright force-pushed the mscoffobj.d branch 4 times, most recently from 4389229 to 3126c9c Compare October 27, 2018 08:00

WalterBright added Severity:Refactoring No semantic changes to code D Conversion labels Oct 27, 2018

WalterBright force-pushed the mscoffobj.d branch from 3126c9c to 8900693 Compare October 27, 2018 09:24

WalterBright added the Review:WIP Work In Progress - not ready for review or pulling label Oct 27, 2018

WalterBright force-pushed the mscoffobj.d branch 2 times, most recently from b4e24e2 to 6061c5f Compare October 27, 2018 09:45

WalterBright force-pushed the mscoffobj.d branch 16 times, most recently from 451371b to 4c41986 Compare October 28, 2018 05:33

WalterBright force-pushed the mscoffobj.d branch 2 times, most recently from fcf54e7 to ce9f335 Compare October 28, 2018 07:06

WalterBright removed the Review:WIP Work In Progress - not ready for review or pulling label Oct 28, 2018

rainers reviewed Oct 28, 2018

View reviewed changes

WalterBright force-pushed the mscoffobj.d branch from ce9f335 to 0fc2e64 Compare October 28, 2018 09:15

rainers reviewed Oct 28, 2018

View reviewed changes

WalterBright force-pushed the mscoffobj.d branch 2 times, most recently from 5eba524 to 5ac0749 Compare October 28, 2018 10:16

thewilsonator reviewed Oct 28, 2018

View reviewed changes

WalterBright force-pushed the mscoffobj.d branch from 5ac0749 to 5e9f424 Compare October 28, 2018 21:11

wilzbach force-pushed the mscoffobj.d branch from 5e9f424 to 2041c6f Compare October 29, 2018 12:52

convert mscoffobj.c to D

2041c6f

andralex approved these changes Nov 2, 2018

View reviewed changes

andralex added the Merge:auto-merge label Nov 2, 2018

WalterBright merged commit ac875bd into dlang:master Nov 2, 2018

WalterBright deleted the mscoffobj.d branch November 2, 2018 22:50

Uh oh!

Conversation

WalterBright commented Oct 27, 2018

Uh oh!

dlang-bot commented Oct 27, 2018

Bugzilla references

Testing this PR locally

Uh oh!

12345swordy commented Oct 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WalterBright commented Oct 28, 2018

Uh oh!

rainers commented Oct 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thewilsonator left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Oct 29, 2018

Uh oh!

andralex left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants