Bug 84974 – WARN_UNUSED_RETURN should be defined for Clang

• Component: Web Template Framework
• Status: ASSIGNED

[Tom Zakrajsek]

Since Clang supports warn_unused_result attribute, the WARN_UNUSED_RETURN macro in Compiler.h (currently in Source/WTF/wtf) should reflect that.

[Tom Zakrajsek]

Created attachment 139021 [details]
test case snippet

[Tom Zakrajsek]

Created attachment 139024 [details]
Patch

[Tom Zakrajsek]

Anders, would you mind reviewing this at your convenience.  Maciej sold you out as the clang expert. :)

[Alexey Proskuryakov]

Comment on attachment 139024 [details]
Patch

What's our general approach to these macros - do we want COMPILER(GCC) to be true in clang (which reportedly defines __GCC__, at least on some systems)? This is related to bug 85018.

[Tom Zakrajsek]

Anders told me in IRC that he does not think clang imply COMPILER(GCC)==true.

[Alexey Proskuryakov]

I think that it does imply that, and we even have code respecting this in Compiler.h:

$ clang -dM -E - < /dev/null
...
#define __GNUC__ 4


#if defined(__GNUC__) && !COMPILER(RVCT)
#define WTF_COMPILER_GCC 1
#define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__)
#define GCC_VERSION_AT_LEAST(major, minor, patch) (GCC_VERSION >= (major * 10000 + minor * 100 + patch))

/* Specific compiler features */
#if !COMPILER(CLANG) && GCC_VERSION_AT_LEAST(4, 6, 0) && defined(__GXX_EXPERIMENTAL_CXX0X__)
#define WTF_COMPILER_SUPPORTS_CXX_NULLPTR 1 
#endif

Note the !COMPILER(CLANG) clause inside defined(__GNUC__)

[Tom Zakrajsek]

I'm holding off on landing this for now.  For reasons that i can't explain, my test case now behaves as if COMPILER(GCC) is already true for clang, and in mapping it out, that's how the code reads too.  This was not the behavior i saw last week, but I see no code change in the logs that would effect this.  I want to understand this file better before making any change.

The definitions are inconsistent either way.  For example, RVCT defines __GNUC__ but we specifically avoid defining COMPILER(GCC) for that case, whereas we appear to define it for other GCC compatible compilers (clang and mingw at least).

Personally, I think the best design would be to have COMPILER(x) values not overlap, but let COMPILER_SUPPORTS(x) handle overlaps where needed.  For now, the behavior I was seeing that prompted the change is not reproducible, so I will spend some more time getting familiar with this code.

[Alexey Proskuryakov]

> Personally, I think the best design would be to have COMPILER(x) values not overlap

That's what I think too, but I've never been involved in designing these macros, and may well overlook something important.