Closed Bug 816487 Opened 12 years ago Closed 12 years ago

WebIDL codegen doesn't support Enum values contain characters outside [a-z_]

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: emk, Assigned: emk)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Is this a spec restriction? Or a limitation of the implementation?
Of the implementation. And it's not the parser but the codegen that has this restriction, no? The reason for that is that we have to produce C++ enum values from the WebIDL enum values. The current way of doing that is very simple, and we just throw on input that would not work with that method, so that we'll fix it if we ever have such input. See the getEnumValueName in Codegen.py. What values are you running into this problem with?
Summary: WebIDL parser doesn't support Enum values contain characters outside [a-z_] → WebIDL codegen doesn't support Enum values contain characters outside [a-z_]
Dashes, I bet.
So would it be a good idea to just convert all non [a-z_] characters to _ as far as the enumeration is concerned, and throw an error if there is a collision in the same enumeration. The chances of that should be rare.
Like this? (obviously uppercase characters should not be replaced with '_', I think.)
Attachment #686842 - Flags: review?(bzbarsky)
Umm... I am not sure if all Unicode characters should be converted to '_', maybe for now just convert ASCII punctuation?
Comment on attachment 686842 [details] [diff] [review] Allow all characters for WebIDL enum Unless we think that the entire pipeline here (Python, compilers, editors, etc) handles non-ASCII, I think we should throw on any non-ASCII char (basically [^ -~] with a comment) and convert everything that's not [A-Za-z_] to '_'. In particular, I have no faith in the number if '_' produced for non-ASCII chars being the same across systems, so would prefer to catch the issue as early as possible.... Sorry for the lag in replying to the part about '/' and '+'; I somehow failed to cc myself.
Attachment #686842 - Flags: review?(bzbarsky) → review-
(In reply to Boris Zbarsky (:bz) from comment #7) > Unless we think that the entire pipeline here (Python, compilers, editors, > etc) handles non-ASCII, I think we should throw on any non-ASCII char > (basically [^ -~] with a comment) Done. > and convert everything that's not > [A-Za-z_] to '_'. Also preserve [0-9], and throw if the value starts with [0-9]. > In particular, I have no faith in the number if '_' > produced for non-ASCII chars being the same across systems, so would prefer > to catch the issue as early as possible.... Throw for consecutive underscores. It's reserved by the C++ spec.
Attachment #687015 - Flags: review?(bzbarsky)
Attachment #686842 - Attachment is obsolete: true
Comment on attachment 687015 [details] [diff] [review] Allow all ASCII characters for WebIDL enum r=me. Thanks!
Attachment #687015 - Flags: review?(bzbarsky) → review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: