Fix Issue 7322 -- Taking address of deprecated functions isn't refused

[edmccard]

http://d.puremagic.com/issues/show_bug.cgi?id=7322

[yebblies]

Please add the test cases that incorrectly fail from the bugzilla report.

[9rnsr]

@edmccard : Can you merge small fail_compilation tests into one module with TEST_OUTPUT feature?

[dnadlinger]

@9rnsr: In case of deprecation warnings, this might work, but in general, I'd prefer them to stay as separate tests. You simply can't test if the compiler exits with a failure for multiple tests in one compilation run, and grepping for all of them in the output might cause problems if we ever change how many error messages are displayed. Hm, maybe using multiple -versions might be a bulletproof way to merge multiple fail_compilation tests together? Would need an extension to d_do_test, though.

[9rnsr]

Yes, in general case that way will not work, but a deprecation error does not change the semantics of its later code, so compiler should warn all deprecation uses in one compilation. Then, squashing tests to one module with TEST_OUTPUT can represent such dmd's diagnostic output specification.

[edmccard]

I'll have to do some more work. I thought it was working in the case "auto x = &y" but it isn't; I should have time to fix it in the next day or two. Then I'll be able to add the test case from the bug report. I will also consolidate the tests and use TEST_OUTPUT.

[edmccard]

Sorry for the delay. The issue was not with auto declarations, but with template functions. At the beginning of SymOffExp::castTo (here: https://github.com/edmccard/dmd/commit/4c497d9232b21b3269e27410c939f4414d7b63c3#L0L1467) was a test:

if (type == t && hasOverloads == 0) 
    return this;

As far as I can tell, this is only reached when template functions are involved? Removing it allows the test case from the bug report, with &rjustify!string, to pass. and it passes the test suite.

Was there a reason to skip the rest of castTo for template functions (or other things where hasOverloads == 0), or is it safe to remove the check?

(Also, the fail_compliation tests have been consolidated to use TEST_OUTPUT)

[donc]

Please remove the import of std.string from the test case. Ideally no tests should use Phobos. Removing all the existing ones would be a pain but we should certainly not introduce any new tests involving it. Your output test includes a deprecation warning, that is definitely going to be removed and will change. If you need to, create a module with a function in it with the same signature as what you're using in the test.

[edmccard]

Rebased, and changed the template function test to use a top-level function in the test source, instead of from std.string.

[ghost]

The import is still here, maybe you forgot to commit before a push -f?

[edmccard]

@AndrejMitrovic, looks like that's what I did. Fixed now.

[donc]

Sorry for dragging this on so long, but could you please add a test corresponding to Kenji's comment 4. Eg, right here add:

void function(int) shouldbeok = &f1;

which shouldn't generate any new errors.

[andralex]

ping?

[andralex]

reping - will close in one week

[9rnsr]

I added bug 7322 fix in #2130 - deprecated attribute check should be deferred until one of overloaded function is exactly picked up.

[WalterBright]

@9rnsr does that mean this should be close?

[9rnsr]

@WalterBright Yes.