[cfe-dev] Storage class as written in the source.

[Douglas Gregor]

On Apr 17, 2010, at 10:02 AM, Enea Zaffanella wrote:

> Douglas Gregor wrote:
>> On Apr 16, 2010, at 11:48 PM, Enea Zaffanella wrote:
> 
> [...]
> 
>>> struct S {
>>> S();
>>> ~S();
>>> operator bool();
>>> };
>>> 
>>> extern S::S() {}
>>> extern S::~S() {}
>>> extern S::operator bool() { return true; }
>> GCC allows it (but it doesn't have any effect on translation), while EDG rejects it. C++ [class.mem]p5 explicitly calls this ill-formed:
>> 	A member shall not be declared with the extern or register storage-class-specifier.
>> We should have an Extension/ExtWarn diagnostic about the storage-class-specifier being redundant and then just drop the storage-class-specifier.
>>> Hence, will it be OK if we leave the extra argument in the constructors of the AST nodes above?
>> I don't think we should model it in the AST because (1) it's not in the language and (2) it has no effect in GCC. Do you disagree?
>> 	- Doug
> 
> Let me try and summarize the state of things, so that you can point out any flaw in my reasoning:
> 
> a) any FunctionDecl node should provide the "storage class as written" info for the purposes of source-based applications such as pretty printers, code style checkers and so on;
> 
> b) the clang AST hierarchy implies that all nodes encoding C++ methods are FunctionDecl nodes, so that they incur no additional memory cost;

Well, it prevents us from re-using those bits for something else, now or in the future.

> c) since GCC accepts the construct, you (wisely) suggest that clang should allow for the same extension and maybe issue corresponding diagnostic.

I'm saying that we should diagnose this violation of the standard and we shouldn't model such ill-formed code in our AST. However, since it's a relatively benign problem (i.e., the presence of such an error would not compromise our AST), I'm okay if the diagnostic is given the ExtWarn/Extension class.

Note that EDG treats this is a hard error, so code that is relying on this extension in GCC is not portable. 

> The combination of a), b) means that the AST is already able to model this info, whereas c) means that you are willing to add the corresponding logic ... so I cannot see where is the problem.
> 
> If the problem is that you do NOT like having an additional argument in the C++ constructor for classes derived from CXXMethodDecl, then we can workaround this while still having this info in the AST.

No, that's not the problem I have. The problem I have is that this appears to be a GCC extension by accident, since it is not documented and has no effect on the translation of the program. The GCC authors probably just forgot to check that part of the standard, like we did. What is the benefit of modeling a GCC accident that is rejected outright by other C++ compilers?

Now, if this extension was *not* an accident and does have some effect on translation, then by all means we should support it and model it in the AST. I haven't seen any evidence that this extension was intentional or has any use.

	- Doug