|Re: [eigen] Eigen2: Assert failure in QR|
[ Thread Index |
| More lists.tuxfamily.org/eigen Archives
- To: eigen@xxxxxxxxxxxxxxxxxxx
- Subject: Re: [eigen] Eigen2: Assert failure in QR
- From: Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
- Date: Fri, 16 Jul 2010 10:45:31 -0400
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:content-type; bh=C+5AUd4ClX6XgFOqxMRW5m8L2b0YoX1lXyRltoZ6LT4=; b=SOW7AilP8D2ayFqnYBI0QucOo8/eNY3Ml8GFnx230zk88kpot4UwcdnH/aRj1fJdcS zasCtJwo9RpGTne2J8THKZhjMK+TFwt8YWVJ1C0Mb4ORW75qxknqXrJIQCisuTCw+V+8 2BL23YMyTFSFuSAnww6iDB1t+6t62wKq/xFQA=
- Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=dMUuwsNyw+nWcAIIDDMshRQTyK/+PrlszRKqw1WRrOzSQELg/utBsS6EVfVgQL2tSJ WZ6c36oRLGf0x0xjbAm0AL7tYVd+qEFuTPpENYG1XQZNBo3pR2uvZ0g975Fay3v+SJQu ytqlCEHDabUXBmvOIimaKqTg4Cif0ADtElm4o=
(Sorry Nick you'll receive this twice)
2010/7/15 Nick Lewycky <nlewycky@xxxxxxxxxx>:
> On 15 July 2010 13:54, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> 2010/7/15 Keir Mierle <mierle@xxxxxxxxx>:
>> > This patch is due to Nick Lewycky.
>> > Fix C++ conformance problems in stable eigen; discovered by clang.
>> > * the 'template' keyword is required when the type is dependent on a
>> > template argument. With 'template' we treat the next identifier as
>> > a template method instead of a member variable and less-than
>> > operator.
>> Cool, surely is an error we've often met and indeed GCC is missing
>> some of them, but in this line,
>> - return this->cast<bool>().cast<int>().sum();
>> + return this->cast<bool>().template cast<int>().sum();
>> it seems to me that the template keyword is also needed in the first
>> cast call, isn't it?
> You stumped me, so I asked the other clang developers and they're not sure
> either. You can add "template" after this-> too if you want to be more
> portable, but it's not clear whether or not it's really needed.
> As implemented, clang will find it because it knows that 'this' has type
> MatrixBase<T> which will never have a dependent base class. I spent a couple
> hours looking through the standard and wasn't able to find anything in there
> that would permit dropping the 'template' keyword in that case, even though
> I was sure there was...
> This is now llvm.org/PR7657 for us to figure it out. Thanks.
Thanks to you for looking into this. Since LLVM accepts the current
version, that's fine for now.
>> > Clang warns on default template argument
>> > int StorageOrder = (unsigned)-1
>> > so cast the whole RHS as int which appears to have been the intent.
>> The patch you attached doesn't seem to do anything about that, right?
> It does, that's the change to Lhs::Flags & RowMajorBit|SparseBit. Here's a
> larger snippet:
> int StorageOrder = ei_is_part<Lhs>::value ? -1 // this is to solve
> ambiguous specializations
> - : int(Lhs::Flags) & (RowMajorBit|SparseBit)
> + : int(Lhs::Flags & (RowMajorBit|SparseBit))
> Note that the literal -1 there was never the problem, to my great confusion
> when I first encountered it!
OK, this is something we've often encountered: we need to cast enum
values to int in ?: to avoid a lot of warnings as they otherwise have
their own enum type. I wish there was custom-typed enums and/or
constexpr in c++98 :-(.