[cfe-commits] r60597 - in /cfe/trunk: include/clang/AST/ include/clang/Basic/ include/clang/Parse/ lib/AST/ lib/CodeGen/ lib/Parse/ lib/Sema/ test/Parser/

[Sebastian Redl]

Douglas Gregor wrote:
> Author: dgregor
> Date: Fri Dec  5 12:15:24 2008
> New Revision: 60597
>
> URL: http://llvm.org/viewvc/llvm-project?rev=60597&view=rev
> Log:
> Representation of template type parameters and non-type template
> parameters, with some semantic analysis:
>   - Template parameters are introduced into template parameter scope
>   - Complain about template parameter shadowing (except in Microsoft mode)
>   
Looks pretty good. I'm a bit worried that this approach means that 
pretty much every bit of Sema code that does some kind of type or 
capability checking has to be aware of template types and defer their 
checking. Is this correct? If so, is there any way we can avoid it? I'm 
envisioning bugs en masse that only appear in template code.
>      // If this scope is a function or contains breaks/continues, remember it.
> -    if (Flags & FnScope)       FnParent = this;
> -    if (Flags & BreakScope)    BreakParent = this;
> -    if (Flags & ContinueScope) ContinueParent = this;
> -    if (Flags & BlockScope)  BlockParent = this;
> -    
> +    if (Flags & FnScope)       	    FnParent = this;
> +    if (Flags & BreakScope)    	    BreakParent = this;
> +    if (Flags & ContinueScope) 	    ContinueParent = this;
> +    if (Flags & BlockScope)    	    BlockParent = this;
> +    if (Flags & TemplateParamScope) TemplateParamParent = this;
>      DeclsInScope.clear();
>   
Who let the tabs in the house? :-)
> --- cfe/trunk/lib/Sema/CMakeLists.txt (original)
> +++ cfe/trunk/lib/Sema/CMakeLists.txt Fri Dec  5 12:15:24 2008
> @@ -18,5 +18,6 @@
>    SemaNamedCast.cpp
>    SemaOverload.cpp
>    SemaStmt.cpp
> +  SemaTemplate.cpp
>    SemaType.cpp
>    )
>   
Very conscientious.
>  
> +  if (PrevDecl && isTemplateParameterDecl(PrevDecl)) {
> +    // Maybe we will complain about the shadowed template parameter.
> +    InvalidDecl 
> +      = InvalidDecl || DiagnoseTemplateParameterShadow(D.getIdentifierLoc(), 
> +						       PrevDecl);
> +    // Just pretend that we didn't see the previous declaration.
> +    PrevDecl = 0;
> +  }
>   
Wouldn't that be better formatted by breaking before or after the ||? At 
least there are a lot of tabs in the final line. Your editor settings 
seem to be messed up.

I don't see any functional issues with the code.

Sebastian