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

C# verbatim character should be removed in error messages #7387

Closed
leppie opened this issue Dec 10, 2015 · 12 comments
Closed

C# verbatim character should be removed in error messages #7387

leppie opened this issue Dec 10, 2015 · 12 comments
Assignees
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings.
Milestone

Comments

@leppie
Copy link
Contributor

leppie commented Dec 10, 2015

Consider the following:

[@X] class F {}

Roslyn reports that @X cannot be found, whereas the legacy compiler reports that X cannot be found.

Where spotted:

  • CS0246
  • CS0579
  • CS0164
  • CA9999
@leppie leppie changed the title Verbatim character should be removed in error messages C# verbatim character should be removed in error messages Dec 10, 2015
@gafter gafter added Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. labels Dec 10, 2015
@gafter
Copy link
Member

gafter commented Dec 10, 2015

This is a matter of using identifier.ValueText in the error message instead of identifier.Text.

@leppie
Copy link
Contributor Author

leppie commented Dec 10, 2015

@gafter Easier said than done :) Consider: @List<@F>. Sadly GenericNameSyntax uses WriteTo which makes things hard.

@gafter
Copy link
Member

gafter commented Dec 10, 2015

We do not support generic attributes today.

@leppie
Copy link
Contributor Author

leppie commented Dec 10, 2015

This is not only about attributes. Consider:

class F : @List<@F> {}

Legacy compiler says: 'List' could not be found
Roslyn says: '@List<@F>' could not be found

Perhaps passing in simpleName instead of where.ToString() for Binder.NotFound will be sufficient for this case. I dont really understand why there is a whereText parameter, goes back to before Git history.

@leppie
Copy link
Contributor Author

leppie commented Dec 10, 2015

Passing in simpleName seems to bring it inline with the legacy compiler. But I am sure there must have been a reason for whereText. Any ideas?

@gafter gafter added the help wanted The issue is "up for grabs" - add a comment if you are interested in working on it label Dec 15, 2015
@gafter gafter added this to the 1.2 milestone Dec 15, 2015
@gafter gafter self-assigned this Dec 15, 2015
@gafter
Copy link
Member

gafter commented Dec 15, 2015

@leppie The reason for the change from the previous (C# 5) version of the compiler is that we rewrote the compiler from scratch.

@gafter
Copy link
Member

gafter commented Dec 15, 2015

Pretty sure this cannot happen with CS0164 as that doesn't have any fill-ins.

@gafter gafter added 3 - Working and removed help wanted The issue is "up for grabs" - add a comment if you are interested in working on it labels Dec 15, 2015
@leppie
Copy link
Contributor Author

leppie commented Dec 15, 2015

@gafter I understand a rewrite is not easy at all. As said in previous comment, it is whereText (which is where.ToString() at the call site) that seems to be odd, and what there is no history for. There must have been some reason for it, even if it was just some workaround at some stage, but from an outsider's point of view, I cannot make that judgement. Else I would try fix it :)

@leppie
Copy link
Contributor Author

leppie commented Dec 15, 2015

@gafter: You are correct :) CS0164 seems to be a mistake. I picked it up from the squiggleText. My bad :)

@leppie
Copy link
Contributor Author

leppie commented Dec 15, 2015

@gafter Does the 3 - Working label imply someone is working on the issue?

@gafter
Copy link
Member

gafter commented Dec 15, 2015

Yeah... almost done.

gafter added a commit to gafter/roslyn that referenced this issue Dec 16, 2015
@gafter gafter added 4 - In Review A fix for the issue is submitted for review. and removed 3 - Working labels Dec 16, 2015
@gafter
Copy link
Member

gafter commented Dec 16, 2015

Blocked by #7516

@gafter gafter added the Blocked label Dec 16, 2015
gafter added a commit to gafter/roslyn that referenced this issue Jan 4, 2016
@gafter gafter closed this as completed in cc10b52 Jan 4, 2016
@gafter gafter removed the 4 - In Review A fix for the issue is submitted for review. label Jan 4, 2016
@gafter gafter removed the Blocked label Jan 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings.
Projects
None yet
Development

No branches or pull requests

2 participants