Bug 315964 - Including with embraced namespace
Summary: Including with embraced namespace
Status: NEW
Alias: None
Product: CDT
Classification: Tools
Component: cdt-indexer (show other bugs)
Version: 7.0   Edit
Hardware: PC Linux
: P3 normal with 8 votes (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact: Jonah Graham CLA
URL:
Whiteboard:
Keywords:
: 329425 418085 450047 508641 527953 544311 (view as bug list)
Depends on:
Blocks: 358815
  Show dependency tree
 
Reported: 2010-06-07 08:22 EDT by Marcelo Taube CLA
Modified: 2020-09-04 15:22 EDT (History)
13 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcelo Taube CLA 2010-06-07 08:22:55 EDT
Build Identifier: 20100218-1602

This is a simply case but very disturbing.
If a file is preprocessor-included with code inside a name space then it is parsed by CDT without context, thus functions defined inside the file are considered global (ie. outside all name spaces). Clicking in the declaration does not help finding the definition

Reproducible: Always

Steps to Reproduce:
1. Create a file called a.h
namespace a {
void foo();
#include "a.inl"
}

2. create a file called a.inl
void foo();

3. now ctrl+click on the declaration of foo, it wont take you to the definition.
Comment 1 Marcelo Taube CLA 2010-06-07 08:24:24 EDT
I forgot to write a function body instead of a semicolon in a.inl
Comment 2 Markus Schorn CLA 2010-06-09 04:56:46 EDT
CDT makes the assumption that a file is only included on file-scope. In case a file is included multiple times from different scopes the interpretation of its content becomes ambiguous. We would not be able to represent this with the current models in place.  
We could solve the simpler case, where a file is always included from the same scope (different from file scope). Currently I am not interested in working on that. As a workaround you may be able to move the inclusion to the file scope.
Comment 3 Marcelo Taube CLA 2010-06-09 05:24:12 EDT
I understand that point of view. But that would not be a problem only of name spaces.
Including a header with different environments will always make ambiguity problems. Different macros defined, different declared specialization of template functions will result in different meaning for a specific header file, and that will always be a problem.

Supposing that the developers of a system cared about the consistency of it, then the header will be included in the same scope each time and i think it is fair to understand the header inclusion in the same way as a C++ compiler would treat it. Don't you think?
Comment 4 Markus Schorn CLA 2010-06-09 06:04:59 EDT
(In reply to comment #3)
> I understand that point of view. But that would not be a problem only of name
> spaces.
> Including a header with different environments will always make ambiguity
> problems. Different macros defined, different declared specialization of
> template functions will result in different meaning for a specific header file,
> and that will always be a problem.
This is true. Headers that are included using different macro dictionaries are
also confusing for developers. In that sense they will always be a problem, too.

> 
> Supposing that the developers of a system cared about the consistency of it,
> then the header will be included in the same scope each time and i think it is
> fair to understand the header inclusion in the same way as a C++ compiler would
> treat it. Don't you think?
Sure and as soon as someone cares enough about it to provide the time needed to implement a solution, it will be there.
Comment 5 Markus Schorn CLA 2010-11-05 04:55:47 EDT
*** Bug 329425 has been marked as a duplicate of this bug. ***
Comment 6 Francois Dr CLA 2011-09-13 10:43:02 EDT
---
NOTE: as adviced in comment of bug 197989, I cross-post to the good bug.

In my following example, there is no namespace, but problem may be similar

------
Hello,

When checking this fix, the following pattern ( currently defective) should be
tested.

You may notice that the fileenum.h cannot be compile alone.
NOTE: this is related to this bug, as, in fact the fileenum.h is loaded
multiple time with different #define set 

please let me know i f i can help you on the fix.

Hp tf ...


#--- file.c ---#
 #include <file.h>

#--- file.h ---#

class foo {
 typdef enum {
     #define set1
     #include <fileenum.h>
     #undef set1
     endofenum
 } menum1 ;
 typdef enum {
     #define set2
     #include <fileenum.h>
     #undef set2
     endofenum2
 } menum2 ;
}

#---file fileenum.h --- #
#ifdef set1
 enum1,
 enum2,
#endif
#ifdef set2
 enum2a,
 enum2b,
#endif
Comment 7 Markus Schorn CLA 2011-10-31 09:38:38 EDT
A solution for this can be implemented as extension to what we have done for bug 197989. To make it happen we need an interaction between parser and preprocessor that informs the preprocessor about the beginning and the end of global declarations. Everything inclusion directive within these points needs to be treated as 'volatile' inclusion.
Comment 8 Marc-André Laperle CLA 2015-02-24 15:36:08 EST
The GDB code base has this problem as well:

in include/gdb/signals.h:

enum gdb_signal
  {
...
#include "gdb/signals.def"
..
  };
Comment 9 Marc-André Laperle CLA 2015-02-24 15:37:35 EST
*** Bug 418085 has been marked as a duplicate of this bug. ***
Comment 10 Nathan Ridge CLA 2016-10-16 11:14:04 EDT
*** Bug 450047 has been marked as a duplicate of this bug. ***
Comment 11 Nathan Ridge CLA 2016-12-03 19:20:04 EST
*** Bug 508641 has been marked as a duplicate of this bug. ***
Comment 12 Nathan Ridge CLA 2016-12-27 18:56:06 EST
When fixing this, we should remove the workaround added in bug 509662.
Comment 13 Marc-André Laperle CLA 2017-01-22 21:28:36 EST
I see this pattern also in libclang.

CIndex.cpp:

class OMPClauseEnqueue {
...
#include "clang/Basic/OpenMPKinds.def"
...
};

  switch (TL.getTypePtr()->getKind()) {
...
  case BuiltinType::Id:
#include "clang/Basic/OpenCLImageTypes.def"
Comment 14 Nathan Ridge CLA 2017-11-30 17:40:51 EST
*** Bug 527953 has been marked as a duplicate of this bug. ***
Comment 15 Volker H. Simonis CLA 2018-02-10 07:09:41 EST
The HotSpot Virtual Machine in the OpenJDK project also heavily uses includes into class definitions to specialize classes for different platforms/architectures. See for example:

http://hg.openjdk.java.net/jdk/jdk/file/tip/src/hotspot/share/runtime/osThread.hpp

which includes (on Linux):

http://hg.openjdk.java.net/jdk/jdk/file/tip/src/hotspot/os/linux/osThread_linux.hpp

like this:

class OSThread: public CHeapObj<mtThread> {
  ...
  // Platform dependent stuff
  #include OS_HEADER(osThread)
  ...
};

OS_HEADER is a macro which expands to one of:

osThread_linux.hpp
osThread_win.hpp
osThread_bsd.hpp
osThread_aix.hpp

depending on the current platform.

It would be really great if you could reconsider support for this admittedly unusual but apparently not uncommon programming style.

Thanks,
Volker
Comment 16 Volker H. Simonis CLA 2018-02-10 08:08:00 EST
I also perfectly understand that the included snippets can not really be reliably parsed and indexed correctly, because in general they can contain code which is only correct within the context where they are included into. But that's not so much of a problem if these "incomplete" headers have syntax errors. More important would be that the compilation units which include them are correctly parsed and indexed WITH the information from the included snippets.
Comment 17 Nathan Ridge CLA 2018-02-10 15:38:44 EST
(In reply to Volker H. Simonis from comment #15)
> It would be really great if you could reconsider support for this admittedly
> unusual but apparently not uncommon programming style.

We haven't decided against supporting this style (if we had, this bug would be closed as "WONTFIX"). It's just that no one has had the time to implement it.

For me personally, the little time I have to contribute to CDT these days is spent fixing regressions (like bug 529396). Anything I have left over I try to spend on forward-looking projects like replacing CDT's homegrown parser with clang's parser using the Language Server Protocol, since it's clear we're not keeping up with new language features like C++17. (By the way, if that project is completed, it will solve this problem too, among many others.)

In the meantime, if someone is interested in contributing a fix for this bug, I would be happy to review and merge it.
Comment 18 Volker H. Simonis CLA 2018-02-10 18:52:16 EST
Thanks for your comments Nathan! Integration with the clang parser using the Language Server Protocol sounds indeed very interesting and promising. Is this tracked by bug 511851 or is there any other link for that project?
Comment 19 Nathan Ridge CLA 2018-02-10 19:19:17 EST
(In reply to Volker H. Simonis from comment #18)
> Thanks for your comments Nathan! Integration with the clang parser using the
> Language Server Protocol sounds indeed very interesting and promising. Is
> this tracked by bug 511851 or is there any other link for that project?

Yes, that is the tracking bug.

The project is in a very early stage. In spite of the bug's current title, I don't think we've committed to using any particular C++ language server at this stage. While ClangD [1] is ostensibly the "official" one, being part of the Clang project, there is also CQuery [2] which seems to be at a more advanced stage in its development.

I've been evaluating CQuery (with an existing client in a different editor) in the past few weeks, and it seems pretty promising.

I also submitted a proposal for a Google Summer of Code project to work on integration between CDT and an LSP-based language server [3]. If you care about this project, and happen to know students looking for programming work this summer, encourage them to apply for it :)

[1] https://clang.llvm.org/extra/clangd.html
[2] https://github.com/cquery-project/cquery/
[3] https://wiki.eclipse.org/Google_Summer_of_Code_2018_Ideas#Eclipse_CDT:_Integration_with_the_Language_Server_Protocol
Comment 20 Nathan Ridge CLA 2019-02-10 13:11:49 EST
*** Bug 544311 has been marked as a duplicate of this bug. ***