r195419 - Make ASTUnit structure stable with NDEBUG

[Alp Toker]

On 22/11/2013 17:48, David Blaikie wrote:
>
>
>
> On Thu, Nov 21, 2013 at 11:49 PM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>     Author: alp
>     Date: Fri Nov 22 01:49:39 2013
>     New Revision: 195419
>
>     URL: http://llvm.org/viewvc/llvm-project?rev=195419&view=rev
>     Log:
>     Make ASTUnit structure stable with NDEBUG
>
>
> I don't believe this is a goal. Is it?

It is:

[cfe-dev] Goal for 3.5: Library-friendly headers
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-November/033345.html


>     ASTUnit instances are allocated infrequently so it's fine to keep
>     this field
>     around in all build configurations.
>
>     Assigns null to silence -Wunused-private-field in Release.
>
>
> That seems like a strange way to silence the warning, can you use an 
> unused or used attribute instead, perhaps?

The unused attribute is less portable and, because it has to be applied 
directly to the declaration, would end up suppressing warnings in all 
build configurations -- not what we want.

Initializing to null silences the warning locally and avoids 
uninitialized values showing up in the debugger.

If this were a hot path like an iterator class there might be a case for 
silencing the warning with cast-to-void instead of assignment, but here 
we're looking at something like once per translation unit. I'd stick 
with this for the readability win alone.

Alp.



>
>     Modified:
>         cfe/trunk/include/clang/Frontend/ASTUnit.h
>         cfe/trunk/lib/Frontend/ASTUnit.cpp
>
>     Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
>     URL:
>     http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=195419&r1=195418&r2=195419&view=diff
>     ==============================================================================
>     --- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
>     +++ cfe/trunk/include/clang/Frontend/ASTUnit.h Fri Nov 22 01:49:39
>     2013
>     @@ -406,9 +406,7 @@ private:
>        /// just about any usage.
>        /// Becomes a noop in release mode; only useful for debug mode
>     checking.
>        class ConcurrencyState {
>     -#ifndef NDEBUG
>          void *Mutex; // a llvm::sys::MutexImpl in debug;
>     -#endif
>
>        public:
>          ConcurrencyState();
>
>     Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
>     URL:
>     http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=195419&r1=195418&r2=195419&view=diff
>     ==============================================================================
>     --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
>     +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Fri Nov 22 01:49:39 2013
>     @@ -2953,7 +2953,7 @@ void ASTUnit::ConcurrencyState::finish()
>
>      #else // NDEBUG
>
>     -ASTUnit::ConcurrencyState::ConcurrencyState() {}
>     +ASTUnit::ConcurrencyState::ConcurrencyState() { Mutex = 0; }
>      ASTUnit::ConcurrencyState::~ConcurrencyState() {}
>      void ASTUnit::ConcurrencyState::start() {}
>      void ASTUnit::ConcurrencyState::finish() {}
>
>
>     _______________________________________________
>     cfe-commits mailing list
>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>

-- 
http://www.nuanti.com
the browser experts