Jump to content

  • Log In with Google      Sign In   
  • Create Account


[c++] returning references and forward declarations


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
11 replies to this topic

#1 Koen   Members   -  Reputation: 455

Like
0Likes
Like

Posted 03 April 2009 - 04:00 AM

I've encountered a compile error in Visual C++. This happens on a regular basis :) but I think this one might be a bug in the compiler. Here's the code
#include <string>

class Product;

class Factory
  {
  public:
    Factory();
    ~Factory();
    Product& Create(const std::string& i_name);
  };

int main(int, const char**)
  {
  Factory factory;
  factory.Create("car"); // <== compile error C2027
  return 0;
  }

The error (see the comment in the code above) is:
error C2027: use of undefined type 'Product'

Obviously this can be fixed easily by including Product.h. But why is this necessary? A reference is being returned, without it ever being used. Shouldn't this compile? (I'm aware that this code cannot be linked) I've tried the same code in Comeau, and there it compiles fine. Am I right in thinking this is a Visual C++ problem, or am I missing something?

Sponsor:

#2 DevFred   Members   -  Reputation: 836

Like
0Likes
Like

Posted 03 April 2009 - 04:14 AM

A factory method that returns a product by reference? That sounds dangerous. Can we see the implementation of Create?

#3 Antheus   Members   -  Reputation: 2393

Like
0Likes
Like

Posted 03 April 2009 - 04:17 AM

This might be a bug, or some obscure behavior, since:
class Product;
class Factory {
public:
Factory() {}
~Factory() {}
Product & Create(const char *) { return *p; }
Product * p;
};

int main(int argc, char* argv[])
{
Factory factory;
Product & p = factory.Create("car");
}
compiles just fine.

My guess would be that compiler assigns the returned value to temporary by value, so it's something like:
Product p = factory.Create("car");

(Product&)factory.Create("car");
also solves the problem.

#4 Koen   Members   -  Reputation: 455

Like
0Likes
Like

Posted 03 April 2009 - 04:24 AM

Quote:
Original post by DevFred
A factory method that returns a product by reference? That sounds dangerous. Can we see the implementation of Create?

Yeah, I should have put this in my original post as well :) I've not actually encountered this in a factory class. I'm aware of the object lifetime issues here. The above code is just an easy example. I should have called the Factory Container instead.



#5 Koen   Members   -  Reputation: 455

Like
0Likes
Like

Posted 03 April 2009 - 04:35 AM

Quote:
Original post by Antheus
This might be a bug, or some obscure behavior, since:*** Source Snippet Removed ***compiles just fine.

Brilliant! I have my warning level at 4 and I'm treating warnings as errors, so I had to type

int main(int, const char**)
{
Factory factory;
Product& p = factory.Create("car");
p; // Oh! Here it is again
return 0;
}


which simply creates the error on the next line :))
So you're probably right about the assignment thing. Strange that it's doing this.
Quote:
(Product&)factory.Create("car");
also solves the problem.

Indeed it does. Thanks a lot.



#6 NotAYakk   Members   -  Reputation: 876

Like
0Likes
Like

Posted 03 April 2009 - 05:14 AM

So, is this a bug in the compiler, a bug in the C++ specs, or actually good behaviour?

#7 MaulingMonkey   Members   -  Reputation: 1556

Like
0Likes
Like

Posted 03 April 2009 - 09:02 AM

This is one of the many false positive warnings provided on level 4 warnings by MSVC. So, D), none of the above.

At least you can disable the individual warnings that tend to repeatedly come up as bogus with /wd#### in your compiler command line options.


Oh, I see, the warning was just for the later snippets.

I'm not sure.

This may have something to do with const references being able to extend the lifetime of an object, although the reference in this case isn't actually const.

#8 kyoryu   Members   -  Reputation: 224

Like
0Likes
Like

Posted 03 April 2009 - 08:29 PM

My guess is that evaluating a reference as an expression results in a copy being put on the stack.

To put a copy of the object on the stack, it would need to know the size of the object, hence the error.

When assigning it to another reference, you're copying it to the destination reference. I don't know what that evaluates to.

Either way, a forward declaration like that is really meant for cases where you're not actually using the forward-declared type (headers, cases where you just pass a pointer around, etc.). Since you're getting a ref and are going to presumably do something with it, you'll probably need to just include the actual header anyway.

#9 Koen   Members   -  Reputation: 455

Like
0Likes
Like

Posted 06 April 2009 - 01:49 AM

Quote:
Original post by kyoryu
My guess is that evaluating a reference as an expression results in a copy being put on the stack.

This is what Antheus said. I've checked and (fortunately) it doesn't actually perform a copy or assignment (for this I obviously had to include the header file). Also, when replacing the reference with a pointer, everything compiles fine with only the forward declaration.
Quote:
Since you're getting a ref and are going to presumably do something with it, you'll probably need to just include the actual header anyway.

I'd hope not! That's the point of forward declarations: if you're not doing anything with the pointer or reference, you don't need the class definition. Besides, you're logic would equally apply to pointers.



#10 kyoryu   Members   -  Reputation: 224

Like
0Likes
Like

Posted 06 April 2009 - 03:13 PM

Quote:
Original post by Koen
I'd hope not! That's the point of forward declarations: if you're not doing anything with the pointer or reference, you don't need the class definition. Besides, you're logic would equally apply to pointers.


Right, if you're not doing anything with the reference. But under what scenario would you return a reference and not do anything with it?

It's definitely a weird piece of behavior, and I don't know if it's defined or undefined. I wouldn't imagine it would be too much of an issue in most real code, as I can't imagine too many places where you'd be needing to evaluate a reference as an expression.

Is there any reason not to just use a pointer?

#11 Koen   Members   -  Reputation: 455

Like
0Likes
Like

Posted 06 April 2009 - 09:36 PM

Quote:
Original post by kyoryu
Right, if you're not doing anything with the reference. But under what scenario would you return a reference and not do anything with it?

The real method where I encountered this, adds a sub-element to a larger structure. The returned reference refers to a configuration object that you can simply ignore if no future re-configuration of the sub-element is required.
Quote:
Is there any reason not to just use a pointer?

The return value cannot be NULL.



#12 phresnel   Members   -  Reputation: 949

Like
0Likes
Like

Posted 07 April 2009 - 12:55 AM

Quote:
Original post by Koen
Quote:
Is there any reason not to just use a pointer?

The return value cannot be NULL.


Funnilly, this can be transformed into an equally valid:

Quote:
Quote:
Is there any reason to use a pointer?

The return value can be NULL.


Having a nullable type is often a cleaner solution then relying on sentinel values. The only problem with that is the potential of callers not testing the return value (certainly, that argument also works for sentinel values). But fortunately, C++ provides the facility to not fear the menaces of null pointer dereferentiation; for example, see boost.optional. It is actually not too hard to write your own optional<T>, for whichever reason.

[Edited by - phresnel on April 7, 2009 8:55:16 AM]




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS