Skip to content

auto functions checker, fix several cases of false warnings#379

Merged
Hackerpilot merged 2 commits intomasterfrom
unknown repository
Dec 6, 2016
Merged

auto functions checker, fix several cases of false warnings#379
Hackerpilot merged 2 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Nov 26, 2016

prevent a few false warnings:

  • assert(0): function not designed to be called.
  • assert(false): function not designed to be called.
  • mixin("return" ...): try to find return in the literal.
  • extern(C) strlen(char*); : so obvious, same for @disable

more cases exist but they are not easily detectable.

Related to dlang/phobos#4850 (comment).
@wilzbach, can you check if the cases that led you to disable the check are handled now ?

@ghost
Copy link
Copy Markdown
Author

ghost commented Nov 26, 2016

Note that it would be possible to skip visit() for a specific version() too. Stddoc seemed to be a problem but the additions made for assert() should do the job.

@ghost ghost changed the title auto functions checker, check assert(false) and strings literal mixin. auto functions checker, fix several cases of false warnings Nov 26, 2016
@Hackerpilot Hackerpilot merged commit f283650 into dlang-community:master Dec 6, 2016
@ghost ghost deleted the autofunc-mixin-asserts branch December 7, 2016 06:46
@wilzbach
Copy link
Copy Markdown
Member

wilzbach commented Dec 8, 2016

@wilzbach, can you check if the cases that led you to disable the check are handled now ?

Wow that managed to catch a lot of false positives - great job :)
It seems we only have the mixin problem left:

std/range/package.d(8943:28)[warn]: Auto function without return statement, prefer an explicit void
std/range/package.d(8948:81)[warn]: Auto function without return statement, prefer an explicit void
std/range/package.d(8955:28)[warn]: Auto function without return statement, prefer an explicit void
std/range/package.d(8960:81)[warn]: Auto function without return statement, prefer an explicit void
std/range/package.d(9126:14)[warn]: Auto function without return statement, prefer an explicit void
std/range/package.d(9133:14)[warn]: Auto function without return statement, prefer an explicit void
std/experimental/ndslice/iteration.d(135:15)[warn]: Auto function without return statement, prefer an explicit void
std/experimental/ndslice/iteration.d(244:15)[warn]: Auto function without return statement, prefer an explicit void

There are just eight occurrences of mixin returns in Phobos, so it might be an option to use the slightly more verbose return mixin(...) alternative there.
Btw you can check the result yourself by running make -f posix.mak style (and setting the flag to enabled of course).

@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 8, 2016

Thanks, yesterday I was browsing all the files in phobos root directory and I didn't get where the hell dscanner was called.
I'll ping again If I manage to handle these cases.

@wilzbach
Copy link
Copy Markdown
Member

wilzbach commented Dec 8, 2016

I'll ping again If I manage to handle these cases.

I don't know whether this is possible because AFAIK is Dscanner unable to expand mixins and verify that they indeed have a return statement. It would be quite fancy to use the D Frontend or the upcoming CTFE engine, but I guess these are larger projects :/

@wilzbach
Copy link
Copy Markdown
Member

wilzbach commented Dec 8, 2016

I'll ping again If I manage to handle these cases.

Btw in case you need a reduced test case, here's a simple one:

	assertAnalyzerWarnings(q{
		auto doStuff(){ mixin(_genSave());}
		private static string _genSave() @safe pure nothrow
        {
            return `import std.stdio;` ~
                   `return true`;
        }
	}, sac);

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.

2 participants