Skip to content

Comments

Fix all DScanner warnings + use -return-exit-code re-enable it#6151

Merged
dlang-bot merged 2 commits intodlang:masterfrom
wilzbach:fix-dscanner
Feb 12, 2018
Merged

Fix all DScanner warnings + use -return-exit-code re-enable it#6151
dlang-bot merged 2 commits intodlang:masterfrom
wilzbach:fix-dscanner

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Feb 10, 2018

tl;dr: DScanner started to segfaulted in December due to the introduction of the AscendingPageAllocator, which used ~this(){}. The DScanner bug has been fixed and now we can fix the DScanner warnings that sneaked in and re-enable in.

See also: #6148

@dlang-bot
Copy link
Contributor

dlang-bot commented Feb 10, 2018

Thanks for your pull request, @wilzbach!

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.

wilzbach
wilzbach commented Feb 10, 2018
View reviewed changes
std/algorithm/iteration.d
static assert(hasDifferentAutodecoding!(T, AliasSeq!(dstring, string)));
static assert(hasDifferentAutodecoding!(T, AliasSeq!(wstring, dstring)));
}
}
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed to for the "public symbol without undocumented example" check.

Copy link
Contributor

@JackStouffer JackStouffer Feb 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. This symbol is not documented and the unit test is not an example.

Copy link
Contributor Author

@wilzbach wilzbach Feb 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning was for template substitute. I don't remember which one, but one of the documentation engines used to dislike different symbols (even if they are private) between declaration and unittest. It looks like this has been fixed, so I can look into updating DScanner... though maybe we can live with this workaround until DScanner has been upgraded?

std/algorithm/iteration.d
}
else static if (isInputRange!Value && !is(CommonType!(ElementType!Value, ElementType!(typeof(substs[0]))) == void))
else static if (isInputRange!Value &&
!is(CommonType!(ElementType!Value, ElementType!(typeof(substs[0]))) == void))
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line > 120

std/algorithm/iteration.d
else
{
static assert(0, "Compile-time substitutions must be elements or ranges of the same type of ` ~ Value.stringof ~ `.");
static assert(0, `Compile-time substitutions must be elements or ranges of the same type of ` ~
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line > 120

std/algorithm/searching.d Outdated
return doesThisStart[0] == withThis;
// specialize for ASCII as to not change previous behavior
else if (withThis <= 0x7F)
else static if (withThis <= 0x7F)
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real bugs :)

std/bigint.d

enum BigInt test1 = BigInt(123);
enum BigInt test2 = plusTwo(test1);
assert(test2 == 125);
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test2 is never used.

std/datetime/systime.d
import core.time;
import std.algorithm.sorting : sort;
import std.typecons : Rebindable;
immutable lt = LocalTime().utcToTZ(0);
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused imports

std/digest/sha.d
sha512_256.put(cast(ubyte[])"");
assert(sha512_256.finish() == cast(ubyte[]) hexString!"c672b8d1ef56ed28ab87c3622c5114069bdd3ad7b8f9737498d0c01ecef0967a");
assert(sha512_256.finish() == cast(ubyte[])
hexString!"c672b8d1ef56ed28ab87c3622c5114069bdd3ad7b8f9737498d0c01ecef0967a");
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long lines

std/net/curl.d

// Default data timeout for Protocols
enum _defaultDataTimeout = dur!"minutes"(2);
private enum _defaultDataTimeout = dur!"minutes"(2);
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undocumented public symbol, it's not too late for this one. It has been only added recently: 91eaa508c4

std/traits.d

static foreach (T; AliasSeq!(void, bool, FloatingPointTypeList, /*ImaginaryTypeList, ComplexTypeList,*/ CharTypeList))
static foreach (T; AliasSeq!(void, bool, FloatingPointTypeList,
/*ImaginaryTypeList, ComplexTypeList,*/ CharTypeList))
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long line

wilzbach
wilzbach commented Feb 10, 2018
View reviewed changes
posix.mak
dscanner: | $(DSCANNER_DIR)/dsc
@echo "Running DScanner"
$(DEBUGGER) -q -ex run -ex bt -batch --args $(DSCANNER_DIR)/dsc --config .dscanner.ini --styleCheck etc std -I.
$(DEBUGGER) -return-child-result -q -ex run -ex bt -batch --args $(DSCANNER_DIR)/dsc --config .dscanner.ini --styleCheck etc std -I.
Copy link
Contributor Author

@wilzbach wilzbach Feb 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: DScanner doesn't seem to be super stable, so imho it's better to keep running it within GDB for a while. The overhead isn't too big and we have done so for the last two months.

@wilzbach wilzbach force-pushed the fix-dscanner branch 2 times, most recently from eb7f72e to 901ff6c Compare February 11, 2018 00:22
@wilzbach
Copy link
Contributor Author

wilzbach commented Feb 11, 2018

image

Yeah, this is finally ready. Sorry for the commit noise.

@dlang-bot dlang-bot merged commit 8d52541 into dlang:master Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andralex andralex Awaiting requested review from andralex

@burner burner Awaiting requested review from burner burner is a code owner

@CyberShadow CyberShadow Awaiting requested review from CyberShadow CyberShadow is a code owner

@DmitryOlshansky DmitryOlshansky Awaiting requested review from DmitryOlshansky DmitryOlshansky is a code owner

@ibuclaw ibuclaw Awaiting requested review from ibuclaw ibuclaw is a code owner

@jmdavis jmdavis Awaiting requested review from jmdavis jmdavis is a code owner

@dnadlinger dnadlinger Awaiting requested review from dnadlinger

@MartinNowak MartinNowak Awaiting requested review from MartinNowak MartinNowak is a code owner

@MetaLang MetaLang Awaiting requested review from MetaLang

@PetarKirov PetarKirov Awaiting requested review from PetarKirov PetarKirov is a code owner

1 more reviewer

@JackStouffer JackStouffer JackStouffer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Merge:auto-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wilzbach @dlang-bot @JackStouffer