Last Comment Bug 750301 - add static asserts internal and xpcom accessible roles are the same
: add static asserts internal and xpcom accessible roles are the same
Status: RESOLVED FIXED
[good first bug][mentor=trev.saunders...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Max Li [:maxli]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-30 08:56 PDT by Trevor Saunders (:tbsaunde)
Modified: 2012-05-11 11:42 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (1.62 KB, patch)
2012-05-04 18:48 PDT, Max Li [:maxli]
tbsaunde+mozbugs: review+
surkov.alexander: feedback+
Details | Diff | Review

Description Trevor Saunders (:tbsaunde) 2012-04-30 08:56:46 PDT
add new file accessible/src/base/RoleAsserts.cpp in it define ROLE() macro as MOZ_STATIC_ASSERT(roles::geckoRole == nsIAccessibleRole::ROLE_ ## geckRole, "internal and xpcom roles differ!");
then include RoleMap.h
Comment 1 Max Li [:maxli] 2012-05-04 18:48:46 PDT
Created attachment 621233 [details] [diff] [review]
Patch v1
Comment 2 alexander :surkov 2012-05-07 04:31:20 PDT
maybe we could introduce general propose Debug.cpp instead? (in case if you don't want to use any existing file). You might want add other assertions there (in case if we do similar things for states for example).
Comment 3 Trevor Saunders (:tbsaunde) 2012-05-07 05:48:48 PDT
(In reply to alexander :surkov from comment #2)
> maybe we could introduce general propose Debug.cpp instead? (in case if you
> don't want to use any existing file). You might want add other assertions
> there (in case if we do similar things for states for example).

I don't feel very strongly either way, on one hand this way we may end up with lots of files, on the other if this file stops compiling you might have a more imediate idea what's wrong.
Comment 4 alexander :surkov 2012-05-07 08:12:17 PDT
I'm just a little bit septic to have couple lines long files. But I'm not sure I like to keep this code under nsAccessibilityService.cpp for example (but why not after all?) so we could introduce generic propose file for stuffs like this. In either case I think I would want to end up with bunch of short files like RoleAssertion.cpp StateAssertion.cpp, RelationAssertion.cpp (etc).
Comment 5 Trevor Saunders (:tbsaunde) 2012-05-08 09:05:09 PDT
(In reply to alexander :surkov from comment #4)
> I'm just a little bit septic to have couple lines long files. But I'm not
> sure I like to keep this code under nsAccessibilityService.cpp for example
> (but why not after all?) so we could introduce generic propose file for
> stuffs like this. In either case I think I would want to end up with bunch
> of short files like RoleAssertion.cpp StateAssertion.cpp,
> RelationAssertion.cpp (etc).

ok, I'm sort of tempted to see what the next use will be so we can understand what grouping actually makes sense instead of what we think will make sense in the future, but sure I'd probably add file Asserts.cpp or BuildAsserts.cpp but it doesn't matter much to me.
Comment 6 alexander :surkov 2012-05-10 04:52:45 PDT
Comment on attachment 621233 [details] [diff] [review]
Patch v1

Review of attachment 621233 [details] [diff] [review]:
-----------------------------------------------------------------

f=me per comment #5
Comment 8 Matt Brubeck (:mbrubeck) 2012-05-11 11:42:11 PDT
https://hg.mozilla.org/mozilla-central/rev/ae9aef30b549

Note You need to log in before you can comment on or make changes to this bug.