Bug 98028 – Put implementation details of StyleBuilder.cpp into anonymous namespace

• Component: CSS
• Status: RESOLVED FIXED

[Yury Semikhatsky]

Otherwise there may be name collision with the rest of WebCore code. E.g. http://queues.webkit.org/results/14075908 where enum value named Image conflicts with WebCore::Image class.

[Yury Semikhatsky]

Created attachment 166469 [details]
Patch

[Pavel Feldman]

Comment on attachment 166469 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166469&action=review

> Source/WebCore/css/StyleBuilder.cpp:896
> +enum BorderImageType { BorderImageImage = 0, BorderImageMask };

BorderImage / BorderMask?

[Yury Semikhatsky]

Comment on attachment 166469 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166469&action=review

>> Source/WebCore/css/StyleBuilder.cpp:896
>> +enum BorderImageType { BorderImageImage = 0, BorderImageMask };
> 
> BorderImage / BorderMask?

Done.

[Yury Semikhatsky]

Created attachment 166472 [details]
Patch

[Yury Semikhatsky]

Committed r130047: <http://trac.webkit.org/changeset/130047>

[Alexey Proskuryakov]

We don't normally use anonymous namespaces, primarily because having a function in anonymous namespace often makes debugging harder.

Also, IDEs don't work well when there are name clashes in the project - an IDE doesn't know which "applyInheritValue" you are searching for. So, you need names that are as unique as possible even when using namespaces.

Why did you choose a unique way to solve this issue instead of just using a more descriptive name?

[Darin Adler]

(In reply to comment #6)
> We don't normally use anonymous namespaces, primarily because having a function in anonymous namespace often makes debugging harder.
> 
> Also, IDEs don't work well when there are name clashes in the project - an IDE doesn't know which "applyInheritValue" you are searching for. So, you need names that are as unique as possible even when using namespaces.
> 
> Why did you choose a unique way to solve this issue instead of just using a more descriptive name?

I was about to say the same thing and ask the same question.

We’ve thought about using anonymous namespaces elsewhere in the past, but rejected them for the reasons Alexey cites above.

[Yury Semikhatsky]

(In reply to comment #6)
> We don't normally use anonymous namespaces, primarily because having a function in anonymous namespace often makes debugging harder.
> 
> Also, IDEs don't work well when there are name clashes in the project - an IDE doesn't know which "applyInheritValue" you are searching for. So, you need names that are as unique as possible even when using namespaces.
> 
Didn't know that we try to avoid anonymous namespaces for better debugging experience. I will remove the anonymous namespace as the more specific names introduced in the change don't conflict with other type names in WebCore. Opened a bug on this: wkbug.com/98124

> Why did you choose a unique way to solve this issue instead of just using a more descriptive name?

The names were change as well and that should be enough to resolve original problem. I added anonymous namespace just to avoid such conflicts in the future, will remove it in favor of better debugging and IDE support.