Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify warning API for custom steps #1384

Closed
sbomer opened this issue Jul 23, 2020 · 2 comments
Closed

Clarify warning API for custom steps #1384

sbomer opened this issue Jul 23, 2020 · 2 comments

Comments

@sbomer
Copy link
Member

sbomer commented Jul 23, 2020

From some discussion with @vitek-karas in #1381:

The docs https://github.com/mono/linker/blob/master/docs/error-codes.md#illink-errors-and-warnings say:

The known codes are in the range 1000 to 6000. Custom steps should avoid using this range not to collide with existing or future error and warning codes.

The ref assembly exposes CreateWarningMessage which has the following check:

			if (!(code > 2000 && code <= 6000))
				throw new ArgumentException ($"The provided code '{code}' does not fall into the warning category, which is in the range of 2001 to 6000 (inclusive).");

Following the docs, we should either invert this check and use a different API for warnings created by the linker internally, or expose a different API for custom steps to use. Some other considerations brought up by @vitek-karas:

I think we need sort of a double treatment here:

  • For the warnings from the linker itself - I strongly feel that having the API only take code is the right approach
  • For custom steps we need the current API which allows everything to be passed in - since we can't build the code->message/version map into the linker

I'm working on a separate PR which will make sure that we only use one message per code (still no API refactoring though). So once that is done we can do the refactoring and create the map.

A quick proposal about the purpose of these APIs:

  • Factor LinkContext.LogWarning to take only a code and message parameters. Only use it for warnings from the linker itself, and don't expose it in the ref assembly. Use an internal helper similar to CreateWarningMessage to validate that the code is in the range [2001-6000].
  • Use CreateWarningMessage for custom steps. It checks that the code is > 6000.

Do we need a way for custom steps to emit known linker-defined warnings?

/cc @mateoatr @marek-safar

@marek-safar
Copy link
Contributor

Do we need a way for custom steps to emit known linker-defined warnings?

No.

The simplest solution could be to just make the check less strict. Essentially changing the logic to something like

	if (code < 2000)
		throw new ArgumentException ($"The warning code '{code}' does not fall into the warning range");

For error handling, we could have different entry points and move the arguments checking there

@marek-safar
Copy link
Contributor

This has been resolved some time ago with the introduction of CreateCustomMessage methods

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

No branches or pull requests

2 participants