Skip to content

Don't require TypeInfo in a minimal runtime if it's not needed#7799

Merged
dlang-bot merged 1 commit intodlang:masterfrom
JinShil:optional_typeinfo
Feb 13, 2018
Merged

Don't require TypeInfo in a minimal runtime if it's not needed#7799
dlang-bot merged 1 commit intodlang:masterfrom
JinShil:optional_typeinfo

Conversation

@JinShil
Copy link
Copy Markdown
Contributor

@JinShil JinShil commented Jan 28, 2018

This is a followup to the following PRs

It is to TypeInfo what the above PRs were for ModuleInfo and Throwable

With this PR, it is now possible to have the same functionality as -betterC (without the -betterC compiler switch) by simply adding the following minimal runtime for executables...

module object;

private alias extern(C) int function(char[][] args) MainFunc;
private extern (C) int _d_run_main(int argc, char** argv, MainFunc mainFunc)
{
    return mainFunc(null);  // assuming `void main()`
}

... or the following minimal runtime for libraries:

module object;

For libraries it would be nice to not require an empty object.d file, but I'm leaving that for another potential PR. UPDATE: See #7825

Again, this will be of interest to users wishing to incrementally or partially port D to new platforms, users hoping to target resource constrained platforms, and users wishing to use D as a library from other languages without druntime.

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request, @JinShil!

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.

{
torig.error(Loc.initial, "`TypeInfo` cannot be used with -betterC");
fatal();
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This block of code was modified to make it consistent with the following in statementsem.d

dmd/src/dmd/statementsem.d

Lines 3626 to 3636 in ec4534e

if (!global.params.useExceptions)
{
tcs.error("Cannot use try-catch statements with -betterC");
return setError();
}
if (!ClassDeclaration.throwable)
{
tcs.error("Cannot use try-catch statements because `object.Throwable` was not declared");
return setError();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we have a valid line for the error?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this error apply for both explicit and implicit uses of RTTI?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this error apply for both explicit and implicit uses of RTTI?

It applies to anything that ends up calling genTypeInfo, which, under the current implementation could be implicit or explicit uses of TypeInfo. For example, trying to do a string concatenation will eventually call runtime hooks like _d_arraycatnTX which takes TypeInfo as one of its arguments. See https://issues.dlang.org/show_bug.cgi?id=18312

IMO all of those runtime hooks should be changed to templates, that don't require TypeInfo, so we shouldn't need TypeInfo for string concatenation as long as the runtime hooks are implemented. That's something I hope to champion after this PR (maybe 1 or 2 more) are merged.

TL;DR: Yes implicit and explicit uses of TypeInfo will trigger the error, but by replacing runtime hooks with templates (later) we may be able to eliminate such errors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, but I'd still feel better if explicit uses got a file/line location to highlight where you're using RTTI.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By "explicit uses" are you referring to typeid? That's the only case I can think of where TypeInfo is explicitly used, and even then, it's indirect. Please let me know if you have something else in mind.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think showing the location of implicit uses is even more desirable, as you cannot grep for these as with explicit uses of typeid. That would mean to pass the location of an expression via semanticTypeInfo and getTypeInfoType.

That will probably also be helpful when generating type info more lazily as in #6561

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll explore this. Marking this as WIP while I try.

// ModuleInfo and exception handling code
// This test ensures an empty main with a struct, built with a minimal runtime,
// does not generate ModuleInfo or exception handling code, and does not
// require TypeInfo
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't you have to modify verify_symbols to check for TypeInfo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, best to be thorough. Done.

@JinShil
Copy link
Copy Markdown
Contributor Author

JinShil commented Feb 10, 2018

Ping @WalterBright, @ZombineDev, @ibuclaw... anyone, really.

{
torig.error(Loc.initial, "`TypeInfo` cannot be used with -betterC");
fatal();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we have a valid line for the error?

{
torig.error(Loc.initial, "`TypeInfo` cannot be used with -betterC");
fatal();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this error apply for both explicit and implicit uses of RTTI?

@JinShil JinShil added the Review:WIP Work In Progress - not ready for review or pulling label Feb 11, 2018
@JinShil
Copy link
Copy Markdown
Contributor Author

JinShil commented Feb 12, 2018

I add a Loc parameter to genTypeInfo for reporting line numbers in the error messages. This increased the diff, of course.

I also added DDoc header comments to the function signatures I modified, and a couple more tests to increase coverage.

There is somewhat of a drawback to this, however. For example, the following code will produce an error message about TypeInfo when it is not apparent, at all, why TypeInfo is needed.

void test()
{
    string s = "abc"
    s = "(" ~ s ~ ")";  // main.d(8): Error: `object.TypeInfo` was not found
}

That is not actually a problem with this PR, but a general problem with D's ubiquitous use of runtime type information (i.e. TypeInfo) for things that are known at compile-time. As I mentioned previously, I intend to champion an effort replace such uses of TypeInfo with templates, which should actually eliminate all such errors, and produce messages that are more meaningful.

cc @ibuclaw @rainers

Copy link
Copy Markdown
Member

@rainers rainers left a comment

Choose a reason for hiding this comment

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

Apart from nits LGTM

src/dmd/e2ir.d Outdated
// This is a hack so we can call postblits on const/immutable objects.
elem *eti = getTypeInfo(tb.unSharedOf().mutableOf(), irs);
AggregateDeclaration ad = cast(AggregateDeclaration)tb;
elem *eti = getTypeInfo(ad.loc, tb.unSharedOf().mutableOf(), irs);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This location is the one of the aggregate declaration, not the using expression. For completeness, setArray should probably receive the expression to forward it here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for the help!

src/dmd/typinf.d Outdated
if (!Type.dtypeinfo)
{
torig.error(Loc.initial, "TypeInfo not found. object.d may be incorrectly installed or corrupt, compile with -v switch");
torig.error(loc, "`object.TypeInfo` was not found");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe add "but implicitly used" to the error message.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@JinShil JinShil removed the Review:WIP Work In Progress - not ready for review or pulling label Feb 13, 2018
@WalterBright
Copy link
Copy Markdown
Member

This produced a regression https://issues.dlang.org/show_bug.cgi?id=18905

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants