Closed Bug 262000 Opened 20 years ago Closed 13 years ago

xpidl should emit adjacent consts of the same type in the same enum

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1498059

People

(Reporter: dbaron, Assigned: jdm)

References

Details

Attachments

(1 file, 1 obsolete file)

I think xpidl should emit adjacent consts of the same time in the same anonymous
enum rather than in separate ones.  This is most likely to make gcc's enumeral
mismatch warnings useful.

Actual results:

 <objdir>/docshell/base/nsIScrollable.h contains (with extra blank lines
 removed):

  enum { ScrollOrientation_X = 1 };                                            
                                   
  enum { ScrollOrientation_Y = 2 };
 ...
  enum { Scrollbar_Auto = 1 }; 
  enum { Scrollbar_Never = 2 }; 
  enum { Scrollbar_Always = 3 };

Expected results:
 <objdir>/docshell/base/nsIScrollable.h contains (with extra blank lines
 removed):

  enum {
    ScrollOrientation_X = 1,                                             
    ScrollOrientation_Y = 2
  };
 ...
  enum {
    Scrollbar_Auto = 1,
    Scrollbar_Never = 2,
    Scrollbar_Always = 3
  };
s/of the same time/of the same type/
Yes, I agree, I wondered why enum's weren't supported in IDL.

One issue is, that enum's aren't guarenteed to be of size int. That might cause
problems for some frozen interfaces.
How is the size of a compile-time constant going to affect frozen interfaces?

This isn't really related to enum support in XPIDL. It's a slightly different
mapping of XPIDL consts from the current one. IMO, the distinction you're
attempting to draw is just too subtle to justify the generation of different
code. It defies the principle of least surprise.
I was thinking the method's arguments would be defined as the enum type. I 
probably went off track with that assumption. If we're just talking using the 
enum to define the constants and not the argument types it's not an issue.
The enums in question here are anonymous; so the type isn't available to be used
in an argument list.
Component: xpidl → XPCOM
QA Contact: pschwartau → xpcom
Attached patch Patch (obsolete) — Splinter Review
Here's my shot at it.  Output for nsITypeAheadFind.h with my patch:

  enum { FIND_FOUND = 0U,
         FIND_NOTFOUND = 1U,
         FIND_WRAPPED = 2U };
Assignee: dbradley → josh
Attachment #441518 - Flags: review?(dbradley)
Attached patch PatchSplinter Review
Removed some cruft.
Attachment #441518 - Attachment is obsolete: true
Attachment #441520 - Flags: review?
Attachment #441518 - Flags: review?(dbradley)
Attachment #441520 - Flags: review? → review?(dbradley)
Looking at the patch now.
Tried building with the patch, but I'm not seeing the changes. Trying to figure out if my build is getting a hold of a pre-built xpidl or something.
(In reply to comment #8)
> Created attachment 441520 [details] [diff] [review]
> Patch
> 
> Removed some cruft.

The patch worked beautifully on my linux build configuration to compile TB3.
(I used relatively recent comm-central, about a week old.)

After applying the patch to xidl_header.c, I recompiled TB3.
I looked for the tell-tale "anonymous" string
in the compilation log, and only found one unrelated warning. 
We eliminated about 40+ (or 50-) warning messages with this single patch from TB3 compilation(!)
Wonderful.

TIA
(In reply to comment #10)
> Tried building with the patch, but I'm not seeing the changes. Trying to figure
> out if my build is getting a hold of a pre-built xpidl or something.

Did you try cloberring ?
Attachment #441520 - Flags: review?(dbradley)
This code has been replaced by pyxpidl.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Re-closing this as a duplicate of bug 1498059, where we actually do something similar to this is pyxpidl now.
Resolution: WONTFIX → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: