[cfe-commits] r140457 - in /cfe/trunk: include/clang/Sema/Initialization.h include/clang/Sema/Overload.h include/clang/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaInit.cpp lib/Sema/SemaOverload.cpp

[Sebastian Redl]

On 26.09.2011, at 16:58, Douglas Gregor wrote:

> 
> On Sep 24, 2011, at 10:48 AM, Sebastian Redl wrote:
> 
>> Author: cornedbee
>> Date: Sat Sep 24 12:48:00 2011
>> New Revision: 140457
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=140457&view=rev
>> Log:
>> Give InitListChecker a verification-only mode, where it neither emits diagnostics nor
>> builds a semantic (structured) initializer list, just reports on whether it can match
>> the given list to the target type.
>> Use this mode for doing init list checking in the initial step of initialization, which
>> will eventually allow us to do overload resolution based on the outcome.
> 
> Cool. Comments below.
> 
>> 
>>  ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
>>                                       AssignmentAction Action,
>> -                                       bool AllowExplicit = false);
>> +                                       bool AllowExplicit = false,
>> +                                       bool Diagnose = true);
>>  ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
>>                                       AssignmentAction Action,
>>                                       bool AllowExplicit,
>> -                                       ImplicitConversionSequence& ICS);
>> +                                       ImplicitConversionSequence& ICS,
>> +                                       bool Diagnose = true);
>>  ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
>>                                       const ImplicitConversionSequence& ICS,
>>                                       AssignmentAction Action,
> 
> Why did you add the Diagnose flag to PerformImplicitConversion? It doesn't seem to be used at all, and probably shouldn't even be there: if the intent is to check whether a conversion would work, wouldn't it be better to make SemaOverload.cpp's TryImplicitConversion available?

It is used in CheckSingleAssignment(). But I didn't know about TryImplicitConversion, and you're right that making it available would be the better choice.

> 
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=140457&r1=140456&r2=140457&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Sep 24 12:48:00 2011
>> @@ -41,6 +41,20 @@
>> using namespace clang;
>> using namespace sema;
>> 
>> +/// \brief Determine whether the use of this declaration is valid, without
>> +/// emitting diagnostics.
>> +bool Sema::CanUseDecl(NamedDecl *D) {
>> +  // See if this is an auto-typed variable whose initializer we are parsing.
>> +  if (ParsingInitForAutoVars.count(D))
>> +    return false;
>> +
>> +  // See if this is a deleted function.
>> +  if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
>> +    if (FD->isDeleted())
>> +      return false;
>> +  }
>> +  return true;
>> +}
> 
> This should also check availability, and return false if the declaration is unavailable.

I just extracted the parts of DiagnoseUseOfDecl that returned a hard error, and I thought I had caught everything. Will re-check.

> 
>> 
>> -  if (!hadError) {
>> +  if (!hadError && !VerifyOnly) {
>>    bool RequiresSecondPass = false;
>>    FillInValueInitializations(Entity, FullyStructuredList, RequiresSecondPass);
> 
> It seems like we do still need to check the value initializations (to make sure there is a suitable default constructor for members that haven't been initialized), although we don't want to build anything.

Good point. Ugh!

>> @@ -1511,6 +1597,9 @@
>>      if (!KnownField && Field->isAnonymousStructOrUnion()) {
>>        if (IndirectFieldDecl *IF =
>>            FindIndirectFieldDesignator(*Field, FieldName)) {
>> +          // In verify mode, don't modify the original.
>> +          if (VerifyOnly)
>> +            DIE = CloneDesignatedInitExpr(SemaRef, DIE);
>>          ExpandAnonymousFieldDesignator(SemaRef, DIE, DesigIdx, IF);
> 
> It would be a little less wasteful, memory-wise, if we could do the expand + clone in one operation, so that we don't end up making a copy of the DesignatedInitExpr storage and then throwing that memory away when we reallocate.

I'll see what I can do.

Thanks for the review.

Sebastian