[cfe-commits] r160634 - in /cfe/trunk: include/clang/AST/CommentSema.h lib/AST/CommentSema.cpp

[Dmitri Gribenko]

Hi Jordan!

Thank you for the review!

On Mon, Jul 23, 2012 at 11:27 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> On Jul 23, 2012, at 10:40 , Dmitri Gribenko <gribozavr at gmail.com> wrote:
> +bool Sema::isFunctionDecl() {
> +  if (IsThisDeclInspected)
> +    return IsFunctionDecl;
> +
> +  inspectThisDecl();
> +  return IsFunctionDecl;
> +}
> +
> +ArrayRef<const ParmVarDecl *> Sema::getParamVars() {
> +  if (IsThisDeclInspected)
> +    return ParamVars;
> +
> +  inspectThisDecl();
> +  return ParamVars;
> +}
>
>
> My personal preference would be to have a single return here, since the body
> is so short (just a call to the inspect… function).
>
> if (!IsThisDeclInspected)
>   inspectThisDecl();
> return ParamVars;

Done in r160689.

> But more importantly, I don't see where IsThisDeclInspected gets reset to
> false.

In the member initializer list:

 Sema::Sema(llvm::BumpPtrAllocator &Allocator, const SourceManager &SourceMgr,
            DiagnosticsEngine &Diags) :
-    Allocator(Allocator), SourceMgr(SourceMgr), Diags(Diags), ThisDecl(NULL) {
+    Allocator(Allocator), SourceMgr(SourceMgr), Diags(Diags), ThisDecl(NULL),
+    IsThisDeclInspected(false) {
 }


Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/