make EpcHelper abstract

[Andrey Mazo, 2013-11-11 16:38:30]

Personally, I would prefer making all or at least one of EpcHelper's methods
pure virtual, so that users get compile-time error instead of runtime one.
The downside, though, is that the compile error won't be so user-friendly and
won't contain detailed explanation what to do next.
Anyway, we have CHANGES.html and RELEASE_NOTES to inform users about API
changes.

If you really want to provide more gentle migration path, we may use
gcc-specific __attribute__((error("The EpcHelper class is now an abstract
class..."))) to mark all EpcHelper's methods instead of making them pure
virtual.

[Nicola Baldo, 2013-11-12 15:50:00]

Hi Andrey, 

First of all, thanks for reviewing the code!

On 2013/11/11 16:38:30, Andrey Mazo wrote:
> Personally, I would prefer making all or at least one of EpcHelper's methods
> pure virtual, so that users get compile-time error instead of runtime one.
> The downside, though, is that the compile error won't be so user-friendly and
> won't contain detailed explanation what to do next.


I deliberately chose using NS_FATAL_ERROR instead of pure virtual methods,
because the compile-time error is not user-friendly. This is a user level API,
so I anticipate that a lot of users would be completely clueless without a clear
error message.


> Anyway, we have CHANGES.html and RELEASE_NOTES to inform users about API
> changes.

I agree in principle (in fact I already updated CHANGES.html), but let's be
realistic: probably 95% of the people posting on ns-3-users don't even know
these file exist. 

 
> If you really want to provide more gentle migration path, we may use
> gcc-specific __attribute__((error("The EpcHelper class is now an abstract
> class..."))) to mark all EpcHelper's methods instead of making them pure
> virtual.

I would be ok with this. My only question is, are gcc-specific attributes
accepted in ns-3 in general?

[Peter Barnes, 2013-11-12 18:25:49]

For other cases, such as unused, we define a NS_<feature> macro, with (at least)
a gcc implementation.  We're a little spotty in defining feature implementations
for other compilers.

For this case, we would need something like 
#define NS_COMPILE_WARNING(msg)

Peter

On 2013/11/12 15:50:00, Nicola Baldo wrote:
> > If you really want to provide more gentle migration path, we may use
> > gcc-specific __attribute__((error("The EpcHelper class is now an abstract
> > class..."))) to mark all EpcHelper's methods instead of making them pure
> > virtual.
> 
> I would be ok with this. My only question is, are gcc-specific attributes
> accepted in ns-3 in general?

[Andrey Mazo, 2013-11-13 09:43:38]

Hi Nicola,

> First of all, thanks for reviewing the code!
Actually, I haven't looked trough the code thoroughly, but just noticed the
NS_FATAL_ERROR()'s.:)

> I deliberately chose using NS_FATAL_ERROR instead of pure virtual methods,
> because the compile-time error is not user-friendly. This is a user level API,
> so I anticipate that a lot of users would be completely clueless without a
clear
> error message.
> 
> > Anyway, we have CHANGES.html and RELEASE_NOTES to inform users about API
> > changes.
> 
> I agree in principle (in fact I already updated CHANGES.html), but let's be
> realistic: probably 95% of the people posting on ns-3-users don't even know
> these file exist. 
We have lots (18) of 'Changes to existing API' in CHANGES.html, none of which
were user-friendly.
Maybe each of them caused tons of complaints and topics on ns-3-users list.
But, I believe, ns-3 users, beginning to learn C++ programming, anyway have to
learn to understand compiler errors sooner or later.

Also, such attempt to not hurt existing users may result in more troubles for
new users.

I suppose, keeping API (and maybe ABI) stable and backwards compatible is a
complex task, which adds significant burden to ns-3 developers, contributors and
maintainers, which may slow down ns-3 development considerably.
Compatibility code size may grow very quickly making it very difficult to
understand and hard to maintain.

[Nicola Baldo, 2013-11-18 12:26:15]

After Andrey's passionate defence of the purist approach, I've revised the patch
so that EpcHelper just uses pure virtual methods (see Patch Set 2).

[Andrey Mazo, 2013-11-18 12:54:09]

On 2013/11/18 12:26:15, Nicola Baldo wrote:
> After Andrey's passionate defence of the purist approach, I've revised the
patch
> so that EpcHelper just uses pure virtual methods (see Patch Set 2).

Thank you very much, Nicola!
Now, +1 from me.:)

[Jinjing, 2013-11-26 07:00:45]

Dear All,

I compiled the latest LTE model with this abstract EpcHelper in ns-3-dev 
(My system is CentOS 6.4), there are several python-binding errors as shown 
below; if I disable ./waf --disable-python, everything is good.
I am clueless now on how to fix this...I guess it is not a python or gcc 
compiler version problem...
I also checked http://www.nsnam.org/wiki/Python_bindings, still not sure 
how to do it...

src/lte/bindings/ns3module.cc: In function ‘int 
_wrap_PyNs3EpcHelper__tp_init__0(PyNs3EpcHelper*, PyObject*, PyObject*, 
PyObject**)’:
src/lte/bindings/ns3module.cc:90009: error: cannot allocate an object of 
abstract type ‘PyNs3EpcHelper__PythonHelper’
src/lte/bindings/ns3module.h:7193: note:   because the following virtual 
functions are pure within ‘PyNs3EpcHelper__PythonHelper’:
./ns3/epc-helper.h:75: note:     virtual void 
ns3::EpcHelper::AddEnb(ns3::Ptr<ns3::Node>, ns3::Ptr<ns3::NetDevice>, 
uint16_t)
./ns3/epc-helper.h:83: note:     virtual void 
ns3::EpcHelper::AddUe(ns3::Ptr<ns3::NetDevice>, uint64_t)
./ns3/epc-helper.h:91: note:     virtual void 
ns3::EpcHelper::AddX2Interface(ns3::Ptr<ns3::Node>, ns3::Ptr<ns3::Node>)
./ns3/epc-helper.h:104: note:     virtual void 
ns3::EpcHelper::ActivateEpsBearer(ns3::Ptr<ns3::NetDevice>, uint64_t, 
ns3::Ptr<ns3::EpcTft>, ns3::EpsBearer)
./ns3/epc-helper.h:115: note:     virtual ns3::Ptr<ns3::Node> 
ns3::EpcHelper::GetPgwNode()
./ns3/epc-helper.h:124: note:     virtual ns3::Ipv4InterfaceContainer 
ns3::EpcHelper::AssignUeIpv4Address(ns3::NetDeviceContainer)
./ns3/epc-helper.h:131: note:     virtual ns3::Ipv4Address 
ns3::EpcHelper::GetUeDefaultGatewayAddress()
src/lte/bindings/ns3module.cc:90016: error: cannot allocate an object of 
abstract type ‘ns3::EpcHelper’
./ns3/epc-helper.h:49: note:   because the following virtual functions are 
pure within ‘ns3::EpcHelper’:
./ns3/epc-helper.h:75: note:     virtual void 
ns3::EpcHelper::AddEnb(ns3::Ptr<ns3::Node>, ns3::Ptr<ns3::NetDevice>, 
uint16_t)
./ns3/epc-helper.h:83: note:     virtual void 
ns3::EpcHelper::AddUe(ns3::Ptr<ns3::NetDevice>, uint64_t)
./ns3/epc-helper.h:91: note:     virtual void 
ns3::EpcHelper::AddX2Interface(ns3::Ptr<ns3::Node>, ns3::Ptr<ns3::Node>)
./ns3/epc-helper.h:104: note:     virtual void 
ns3::EpcHelper::ActivateEpsBearer(ns3::Ptr<ns3::NetDevice>, uint64_t, 
ns3::Ptr<ns3::EpcTft>, ns3::EpsBearer)
./ns3/epc-helper.h:115: note:     virtual ns3::Ptr<ns3::Node> 
ns3::EpcHelper::GetPgwNode()
./ns3/epc-helper.h:124: note:     virtual ns3::Ipv4InterfaceContainer 
ns3::EpcHelper::AssignUeIpv4Address(ns3::NetDeviceContainer)
./ns3/epc-helper.h:131: note:     virtual ns3::Ipv4Address 
ns3::EpcHelper::GetUeDefaultGatewayAddress()
src/lte/bindings/ns3module.cc: In function ‘int 
_wrap_PyNs3EpcHelper__tp_init__1(PyNs3EpcHelper*, PyObject*, PyObject*, 
PyObject**)’:
src/lte/bindings/ns3module.cc:90040: error: cannot allocate an object of 
abstract type ‘PyNs3EpcHelper__PythonHelper’
src/lte/bindings/ns3module.h:7193: note:   since type 
‘PyNs3EpcHelper__PythonHelper’ has pure virtual functions
src/lte/bindings/ns3module.cc:90047: error: cannot allocate an object of 
abstract type ‘ns3::EpcHelper’
./ns3/epc-helper.h:49: note:   since type ‘ns3::EpcHelper’ has pure virtual 
functions
src/lte/bindings/ns3module.cc: In function ‘PyObject* 
_wrap_PyNs3EpcHelper__copy__(PyNs3EpcHelper*)’:
src/lte/bindings/ns3module.cc:90292: error: cannot allocate an object of 
abstract type ‘ns3::EpcHelper’
./ns3/epc-helper.h:49: note:   since type ‘ns3::EpcHelper’ has pure virtual 
functions



On Monday, November 18, 2013 4:54:10 AM UTC-8, Andrey Mazo wrote:
>
> On 2013/11/18 12:26:15, Nicola Baldo wrote: 
> > After Andrey's passionate defence of the purist approach, I've revised 
> the patch 
> > so that EpcHelper just uses pure virtual methods (see Patch Set 2). 
>
> Thank you very much, Nicola! 
> Now, +1 from me.:) 
>
> https://codereview.appspot.com/24460043/ 
>

[neeruchandana007_gmail.com, 2014-06-05 15:20:32]

Hi Jinjing Jiang, 

  Did you get the solution for your problem. If yes, Can you please tell 
me. I also got the same error.

  Thanks.