Library compile failures with Intel C++ 18 and -std=c++17

Issue #182 wontfix
Dan Bonachea created an issue

The example below uses the system nightly install of UPC++ on cori/haswell with PrgEnv-intel/6.0.4 and demonstrates that things break in horrible ways when the user compiles a simple application code with the addition of -std=c++17.

Note this is distinct from similar issue #181 because the symptom is a typechecking failure, and occurs even if one attempts to build the UPC++ library with icc -std=c++17 using the system default Intel C v18.0.1.163.

Preliminary experiments with Intel C++ 19.0.0.117 indicate the problem might be fixed in that version.

We should triage this failure further for our portability milestone, determine if there is any tractable/acceptable workaround, and either apply a workaround or document the problem.

{cori-hsw ~/UPC/upcxx/example/prog-guide} module load upcxx/nightly 
## Loaded 'PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26' based on currently loaded modules.
## The selected build is for target CPU "haswell", and thus if
## you change craype-{haswell,mic-knl} modules then you should
## 'module unload upcxx' and reload to get the correct upcxx module.
##
## WARNING: nightly snapshots typically live for only 48 hours and
## are not intended for general use due to potential instability.

{cori-hsw ~/UPC/upcxx} module list
Currently Loaded Modulefiles:
  1) modules/3.2.10.6                                 10) dmapp/7.1.1-6.0.7.0_34.3__g5a674e0.ari           19) craype-haswell
  2) nsg/1.2.0                                        11) gni-headers/5.0.12.0-6.0.7.0_24.1__g3b1768f.ari  20) cray-mpich/7.7.0
  3) intel/18.0.1.163                                 12) xpmem/2.2.15-6.0.7.1_5.8__g7549d06.ari           21) altd/2.0
  4) craype-network-aries                             13) job/2.2.3-6.0.7.0_44.1__g6c4e934.ari             22) darshan/3.1.4
  5) craype/2.5.14                                    14) dvs/2.7_2.2.113-6.0.7.1_7.1__g1bbc03e            23) bupc/2.26.0
  6) cray-libsci/18.03.1                              15) alps/6.6.43-6.0.7.0_26.4__ga796da3.ari           24) git/2.15.1
  7) udreg/2.3.2-6.0.7.0_33.18__g5196236.ari          16) rca/2.2.18-6.0.7.0_33.3__g2aa4f39.ari            25) gcc/7.3.0
  8) ugni/6.0.14.0-6.0.7.0_23.1__gea11d3d.ari         17) atp/2.1.1                                        26) upcxx/nightly
  9) pmi/5.0.13                                       18) PrgEnv-intel/6.0.4

{cori-hsw ~/UPC/upcxx/example/prog-guide} which upcxx
/usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/bin/upcxx

{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx --version
UPC++ version 20180901 upcxx-2018.3.3-159-g57a9814
Copyright (c) 2018, The Regents of the University of California,
through Lawrence Berkeley National Laboratory.
http://upcxx.lbl.gov

icpc (ICC) 18.0.1 20171018
Copyright (C) 1985-2017 Intel Corporation.  All rights reserved.

{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx -g -std=c++14 compute-pi.cpp
{cori-hsw ~/UPC/upcxx/example/prog-guide} upcxx -g -std=c++17 compute-pi.cpp 
In file included from /opt/gcc/7.3.0/snos/include/g++/string(52),
                 from /opt/gcc/7.3.0/snos/include/g++/bits/locale_classes.h(40),
                 from /opt/gcc/7.3.0/snos/include/g++/bits/ios_base.h(41),
                 from /opt/gcc/7.3.0/snos/include/g++/ios(42),
                 from /opt/gcc/7.3.0/snos/include/g++/ostream(38),
                 from /opt/gcc/7.3.0/snos/include/g++/iostream(39),
                 from compute-pi.cpp(1):
/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h(113): error: basic_string_view is not a template
        typedef basic_string_view<_CharT, _Traits> __sv_type;
                ^

In file included from /opt/gcc/7.3.0/snos/include/g++/bits/hashtable.h(37),
                 from /opt/gcc/7.3.0/snos/include/g++/unordered_map(47),
                 from /opt/gcc/7.3.0/snos/include/g++/functional(60),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/utility.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future/core.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future.hpp(4),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/backend.hpp(5),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/allocate.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/upcxx.hpp(8),
                 from compute-pi.cpp(4):
/opt/gcc/7.3.0/snos/include/g++/bits/node_handle.h(131): error: optional is not a template
        optional<_NodeAlloc>            _M_alloc;
        ^

In file included from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/utility.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future/core.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future.hpp(4),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/backend.hpp(5),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/allocate.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/upcxx.hpp(8),
                 from compute-pi.cpp(4):
/opt/gcc/7.3.0/snos/include/g++/functional(139): error: class template "std::_Mem_fn_traits" has already been defined
  _GLIBCXX_MEM_FN_TRAITS(noexcept, true_type, true_type)
  ^

In file included from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/utility.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future/core.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future.hpp(4),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/backend.hpp(5),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/allocate.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/upcxx.hpp(8),
                 from compute-pi.cpp(4):
/opt/gcc/7.3.0/snos/include/g++/functional(140): error: class template "std::_Mem_fn_traits" has already been defined
  _GLIBCXX_MEM_FN_TRAITS(& noexcept, true_type, false_type)
  ^

In file included from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/utility.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future/core.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/future.hpp(4),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/backend.hpp(5),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/allocate.hpp(8),
                 from /usr/common/ftg/upcxx/nightly/hsw/intel/PrgEnv-intel-6.0.4-18.0.1.163-2018.11.26/upcxx.debug.gasnet_seq.aries/include/upcxx/upcxx.hpp(8),
                 from compute-pi.cpp(4):
/opt/gcc/7.3.0/snos/include/g++/functional(141): error: class template "std::_Mem_fn_traits" has already been defined
  _GLIBCXX_MEM_FN_TRAITS(&& noexcept, false_type, true_type)
  ^

/opt/gcc/7.3.0/snos/include/g++/type_traits(1545): error: class "std::__is_convertible_helper<char *const &, <error-type>, false>" has no member class "type"
      : public __is_convertible_helper<_From, _To>::type
                                                    ^
          detected during:
            instantiation of class "std::is_convertible<_From, _To> [with _From=char *const &, _To=<error-type>]" at line 149
            instantiation of class "std::__and_<_B1, _B2, _B3, _Bn...> [with _B1=std::is_convertible<char *const &, <error-type>>, _B2=std::__not_<std::is_convertible<char *const *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> *>>, _B3=std::__not_<std::is_convertible<char *const &, const char *>>, _Bn=<>]" at line 119 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of type "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_If_sv<char *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &> [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 2184 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::replace [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" based on template argument <char *> at line 1614 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 5959 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> std::operator+(std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 351 of "/opt/gcc/7.3.0/snos/include/g++/system_error"

/opt/gcc/7.3.0/snos/include/g++/type_traits(1545): error: not a class or struct name
      : public __is_convertible_helper<_From, _To>::type
               ^
          detected during:
            instantiation of class "std::is_convertible<_From, _To> [with _From=char *const &, _To=<error-type>]" at line 149
            instantiation of class "std::__and_<_B1, _B2, _B3, _Bn...> [with _B1=std::is_convertible<char *const &, <error-type>>, _B2=std::__not_<std::is_convertible<char *const *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> *>>, _B3=std::__not_<std::is_convertible<char *const &, const char *>>, _Bn=<>]" at line 119 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of type "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_If_sv<char *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &> [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 2184 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::replace [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" based on template argument <char *> at line 1614 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 5959 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> std::operator+(std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 351 of "/opt/gcc/7.3.0/snos/include/g++/system_error"

/opt/gcc/7.3.0/snos/include/g++/type_traits(149): error: class "std::is_convertible<char *const &, <error-type>>" has no member "value"
      : public conditional<_B1::value, __and_<_B2, _B3, _Bn...>, _B1>::type
                                ^
          detected during:
            instantiation of class "std::__and_<_B1, _B2, _B3, _Bn...> [with _B1=std::is_convertible<char *const &, <error-type>>, _B2=std::__not_<std::is_convertible<char *const *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> *>>, _B3=std::__not_<std::is_convertible<char *const &, const char *>>, _Bn=<>]" at line 119 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of type "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_If_sv<char *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &> [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 2184 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::replace [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" based on template argument <char *> at line 1614 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 5959 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> std::operator+(std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 351 of "/opt/gcc/7.3.0/snos/include/g++/system_error"

/opt/gcc/7.3.0/snos/include/g++/type_traits(2476): error: class "std::enable_if<false, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &>" has no member "type"
      using enable_if_t = typename enable_if<_Cond, _Tp>::type;
                                                          ^
          detected during:
            instantiation of type "std::enable_if_t<false, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &>" at line 120 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of type "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::_If_sv<char *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &> [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 2184 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::replace [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" based on template argument <char *> at line 1614 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::size_type, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 5959 of "/opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h"
            instantiation of "std::__cxx11::basic_string<_CharT, _Traits, _Alloc> std::operator+(std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc> &&) [with _CharT=char, _Traits=std::char_traits<char>, _Alloc=std::allocator<char>]" at line 351 of "/opt/gcc/7.3.0/snos/include/g++/system_error"

compilation aborted for compute-pi.cpp (code 2)

Comments (5)

  1. Dan Bonachea reporter

    I've reproduced the problem on JLSE's (non-Cray) Linux cluster with Intel C++ 18.0.1.163, and confirmed that the problem does not appear to affect Intel C++ 19.0.0.117

  2. Paul Hargrove

    Dan,

    Intel's compliers are entirely dependent on the C++ standard library provided with the g++ in your $PATH.
    That is evidenced by the /opt/gcc/7.3.0/snos/include/g++ in all those messages.
    So, it is possible (not certain by any means) that this is an incompatibility between icc-18 and gcc-7.3.0 at the C++17 language level.

    I believe you and I have already triaged that incompatibility once with respect to the UPC/C++ interop tests (see here) and found (as you have) that icc-19 did not have the problem.
    There is absolutely no UPC++ code in that compilation failure, but at least the basic_string_view error looks to be the same.

  3. Dan Bonachea reporter

    We suspect this is a defect in Intel 18's support for -std=c++17 with the g++-7 libstdc++ headers

  4. Dan Bonachea reporter

    I've replicated this problem on cori using a simple non-UPC++ program that just #includes <iostream>:

    {cori-hsw ~/UPC/code} module list
    Currently Loaded Modulefiles:
      1) modules/3.2.10.6                                  8) ugni/6.0.14.0-6.0.7.1_3.13__gea11d3d.ari         15) alps/6.6.43-6.0.7.1_5.46__ga796da32.ari          22) altd/2.0
      2) nsg/1.2.0                                         9) pmi/5.0.14                                       16) rca/2.2.18-6.0.7.1_5.48__g2aa4f39.ari            23) darshan/3.1.4
      3) intel/18.0.1.163                                 10) dmapp/7.1.1-6.0.7.1_6.2__g45d1b37.ari            17) atp/2.1.3                                        24) git/2.15.1
      4) craype-network-aries                             11) gni-headers/5.0.12.0-6.0.7.1_3.11__g3b1768f.ari  18) PrgEnv-intel/6.0.4                               25) upcxx/2019.3.2
      5) craype/2.5.15                                    12) xpmem/2.2.15-6.0.7.1_5.11__g7549d06.ari          19) craype-haswell                                   26) bupc-narrow/2019.4.2
      6) cray-libsci/18.07.1                              13) job/2.2.3-6.0.7.1_5.44__g6c4e934.ari             20) cray-mpich/7.7.3                                 27) upcxx-bupc-narrow/2019.3.2
      7) udreg/2.3.2-6.0.7.1_5.13__g5196236.ari           14) dvs/2.7_2.2.120-6.0.7.1_12.1__g74cb2cc4          21) gcc/7.3.0
    
    {cori-hsw ~/UPC/code} cat hello.cc
    #include <iostream>
    
    int main() {
      std::cout << "hi" << std::endl;
      return 0;
    }
    
    {cori-hsw ~/UPC/code} cc -std=c++14 hello.cc
    [works]
    {cori-hsw ~/UPC/code} cc -std=c++17 hello.cc 
    In file included from /opt/gcc/7.3.0/snos/include/g++/string(52),
                     from /opt/gcc/7.3.0/snos/include/g++/bits/locale_classes.h(40),
                     from /opt/gcc/7.3.0/snos/include/g++/bits/ios_base.h(41),
                     from /opt/gcc/7.3.0/snos/include/g++/ios(42),
                     from /opt/gcc/7.3.0/snos/include/g++/ostream(38),
                     from /opt/gcc/7.3.0/snos/include/g++/iostream(39),
                     from hello.cc(1):
    /opt/gcc/7.3.0/snos/include/g++/bits/basic_string.h(113): error: basic_string_view is not a template
            typedef basic_string_view<_CharT, _Traits> __sv_type;
                    ^
    [snip]
    {cori-hsw ~/UPC/code} module switch intel intel/19.0.0.117 
    {cori-hsw ~/UPC/code} cc -std=c++17 hello.cc
    [works]
    

    So bottom-line this is a defect in Intel C++ that has nothing to do with UPC++. Intel versions prior to version 18.0.2 are incompatible with the gcc/7.3.0 std headers in std=c++17 mode, and this is not something we can fix. Recommended workaround is to use a newer Intel compiler.

  5. Log in to comment