standardese/cppast

Errors with "circular" includes

Reen opened this issue · 14 comments

Reen commented
  • cppast version: e5fa6b7
  • parser: libclang_parser
  • clang version: 14.0.0

Hi Jonathan,
thanks for making such a great library. I'm trying to get it running with my code base, but encountered another error.
I have two classes that reference each other. Parsing these files with cppast gives errors as below

Input:

N/A.hpp:

#pragma once

#include <cmath>
#include <cstdint>

namespace N{

struct B;

struct A {
	float a;
	N::B make_b() const;
};

}

#include <N/B.hpp>

inline N::B N::A::make_b() const{
	return {};
}

N/B.hpp

#pragma once

#include <cmath>
#include <cassert>

namespace N{

struct A;

struct B{
    float b;
    N::A make_a() const;
};
}

#include <N/A.hpp>

inline N::A N::B::make_a() const
{
    return {};
}

Input flags: -v -I /abs/path/to/base_of_N/ /abs/path/to/N/A.hpp

Output:

[preprocessor] [debug] /home/.../N/A.hpp:3: parsing include 'cmath'
[preprocessor] [debug] /home/.../N/A.hpp:4: parsing include 'cstdint'
[preprocessor] [debug] /home/.../N/A.hpp:17: parsing include 'N/B.hpp'
[preprocessor] [debug] /home/.../N/A.hpp:4: parsing include 'cstdint'
[preprocessor] [debug] /home/.../N/A.hpp:17: parsing include 'N/B.hpp'
[preprocessor] [warning] /home/.../N/A.hpp:4: unable to retrieve full path for include 'cstdint' (please file a bug report)
[preprocessor] [warning] /home/.../N/A.hpp:17: unable to retrieve full path for include 'N/B.hpp' (please file a bug report)
[libclang] [error] ./N/A.hpp:10: redefinition of 'A'
[libclang] [error] ./N/A.hpp:10: redefinition of 'A'
[libclang] [error] ./N/A.hpp:19: redefinition of 'make_b'
[libclang] [error] ./N/B.hpp:18: redefinition of 'make_a'
[libclang] [error] ./N/A.hpp:10: redefinition of 'A'
AST for './N/A.hpp':

The output differs if I pass a relative path to the input file:

Input flags: -v -I /abs/path/to/base_of_N/ N/A.hpp

Output:

[preprocessor] [debug] N/A.hpp:3: parsing include 'cmath'
[preprocessor] [debug] N/A.hpp:4: parsing include 'cstdint'
[preprocessor] [debug] N/A.hpp:17: parsing include 'N/B.hpp'
[libclang] [error] ./N/A.hpp:10: redefinition of 'A'
AST for './N/A.hpp':
foonathan commented

... I get that this is a bug in the tool, but why do you have two headers that mutual include each other?!

Reen commented

The example shows the reason why. Both, A and B, have a method that turns the one into the other. In order to achieve this, there is only the way shown. I know that there might be other architectural ways like providing functions outside both classes that do this, but in our code base we chose this way because it allows for writing code that is easy to understand.

foonathan commented

I know that there might be other architectural ways like providing functions outside both classes that do this, but in our code base we chose this way because it allows for writing code that is easy to understand.

As a compiler author, it's not my place to criticize your code, but why not define it in the source file instead? Or put both classes in the same header?

Anyways, I've added a workaround for the issue.

Reen commented

but why not define it in the source file instead?

You are right, this would be the normal way to do it. But, the library is a header-only library.

Or put both classes in the same header?

This would work. But we try to keep one class per file for various reasons.

Anyway, the bugfix only solved part of the problem. It still says:

[preprocessor] [debug] N/A.hpp:3: parsing include 'cmath'
[preprocessor] [debug] N/A.hpp:4: parsing include 'cstdint'
[preprocessor] [debug] N/A.hpp:17: parsing include 'N/B.hpp'
AST for './N/A.hpp':

if I pass a relative path to N/A.hpp and

[preprocessor] [debug] /abs/path/to/base_of_N/N/A.hpp:3: parsing include 'cmath'
[preprocessor] [debug] /abs/path/to/base_of_N/N/A.hpp:4: parsing include 'cstdint'
[preprocessor] [debug] /abs/path/to/base_of_N/N/A.hpp:17: parsing include 'N/B.hpp'
[preprocessor] [debug] /abs/path/to/base_of_N/N/A.hpp:4: parsing include 'cstdint'
[preprocessor] [debug] /abs/path/to/base_of_N/N/A.hpp:17: parsing include 'N/B.hpp'
[preprocessor] [warning] /abs/path/to/base_of_N/N/A.hpp:4: unable to retrieve full path for include 'cstdint' (please file a bug report)
[preprocessor] [warning] /abs/path/to/base_of_N/N/A.hpp:17: unable to retrieve full path for include 'N/B.hpp' (please file a bug report)
[libclang] [error] ./N/A.hpp:10: redefinition of 'A'
[libclang] [error] ./N/A.hpp:23: redefinition of 'make_a'
[libclang] [error] ./N/A.hpp:19: redefinition of 'make_b'
AST for './N/A.hpp':

If I pass an absolute path to the file.

foonathan commented

Okay, now I should have fixed it :D

deadlocklogic commented

After fixing #150, after including any standard header like for example<string> the compiler generates redefinition errors, and all its global members are dumped into the parsed files which is really annoying and unexpected.
The test file contains a simple dummy function:
test.h

#pragma once

#include <string>

static void Dummy(std::string value) {
}

compiler errors:

[libclang] [error] C:/Users/User/Desktop/test.h:41: redefinition of '_Char_traits'
[libclang] [error] C:/Users/User/Desktop/test.h:221: redefinition of '_WChar_traits'
[libclang] [error] C:/Users/User/Desktop/test.h:333: redefinition of 'char_traits'
[libclang] [error] C:/Users/User/Desktop/test.h:336: redefinition of 'char_traits<char16_t>'
[libclang] [error] C:/Users/User/Desktop/test.h:339: redefinition of 'char_traits<char32_t>'
[libclang] [error] C:/Users/User/Desktop/test.h:342: redefinition of 'char_traits<wchar_t>'
[libclang] [error] C:/Users/User/Desktop/test.h:346: redefinition of 'char_traits<unsigned short>'
[libclang] [error] C:/Users/User/Desktop/test.h:356: redefinition of '_Narrow_char_traits'
[libclang] [error] C:/Users/User/Desktop/test.h:480: redefinition of 'char_traits<char>'
[libclang] [error] C:/Users/User/Desktop/test.h:488: redefinition of '_Insert_string'
[libclang] [error] C:/Users/User/Desktop/test.h:537: redefinition of '_Char_traits_eq'
[libclang] [error] C:/Users/User/Desktop/test.h:546: redefinition of '_Char_traits_lt'
[libclang] [error] C:/Users/User/Desktop/test.h:557: redefinition of '_Can_memcmp_elements_with_pred<_Elem, _Elem, _Char_traits_eq<char_traits<_Elem>>>'
[libclang] [error] C:/Users/User/Desktop/test.h:563: redefinition of '_Lex_compare_memcmp_classify_pred<_Elem, _Elem, _Char_traits_lt<char_traits<_Elem>>>'
[libclang] [error] C:/Users/User/Desktop/test.h:575: redefinition of '_Traits_equal'
[libclang] [error] C:/Users/User/Desktop/test.h:582: redefinition of '_Traits_compare'
[libclang] [error] C:/Users/User/Desktop/test.h:603: redefinition of '_Traits_find'
[libclang] [error] C:/Users/User/Desktop/test.h:637: redefinition of '_Traits_find_ch'
[libclang] [error] C:/Users/User/Desktop/test.h:651: redefinition of '_Traits_rfind'
[libclang parser] [warning] C:/Users/User/Desktop/test.h:557: unhandled cursor of kind 'UnexposedDecl'
[libclang parser] [warning] C:/Users/User/Desktop/test.h:2325: unhandled cursor of kind 'UnexposedDecl'
[libclang parser] [warning] C:/Users/User/Desktop/test.h:2403: unhandled cursor of kind 'UnexposedDecl'

dumped function name/signature using a visitor:

std::_Insert_string(basic_ostream<_Elem, _Traits>&,_Elem const* const,_SizeT const)
std::_Traits_equal(_Traits_ptr_t<_Traits> const,const size_t,_Traits_ptr_t<_Traits> const,const size_t)
std::_Traits_compare(_Traits_ptr_t<_Traits> const,const size_t,_Traits_ptr_t<_Traits> const,const size_t)
std::_Traits_find(_Traits_ptr_t<_Traits> const,const size_t,const size_t,_Traits_ptr_t<_Traits> const,const size_t)
std::_Traits_find_ch(_Traits_ptr_t<_Traits> const,const size_t,const size_t,_Traits_ch_t<_Traits> const)
std::_Traits_rfind(_Traits_ptr_t<_Traits> const,const size_t,const size_t,_Traits_ptr_t<_Traits> const,const size_t)
std::_Traits_rfind_ch(_Traits_ptr_t<_Traits> const,const size_t,const size_t,_Traits_ch_t<_Traits> const)
std::_Traits_find_first_of(int const,const size_t,const size_t,int const,const size_t)
std::_Traits_find_last_of(int const,const size_t,const size_t,int const,const size_t)
std::_Traits_find_first_not_of(int const,const size_t,const size_t,int const,const size_t)
std::_Traits_find_not_ch(int const,const size_t,const size_t,int const)
std::_Traits_find_last_not_of(int const,const size_t,const size_t,int const,const size_t)
std::_Traits_rfind_not_ch(int const,const size_t,const size_t,int const)
std::operator+(typename _String_const_iterator<_Mystr>::difference_type,_String_const_iterator<_Mystr>)
std::operator+(typename _String_iterator<_Mystr>::difference_type,_String_iterator<_Mystr>)
std::_Xlen_string()
std::swap(basic_string<_Elem, _Traits, _Alloc>&,basic_string<_Elem, _Traits, _Alloc>&)
std::operator+(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator+(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator+(_Elem const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator+(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator+(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const)
std::operator+(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc>&&)
std::operator+(basic_string<_Elem, _Traits, _Alloc>&&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator+(basic_string<_Elem, _Traits, _Alloc>&&,basic_string<_Elem, _Traits, _Alloc>&&)
std::operator+(_Elem const* const,basic_string<_Elem, _Traits, _Alloc>&&)
std::operator+(_Elem const,basic_string<_Elem, _Traits, _Alloc>&&)
std::operator+(basic_string<_Elem, _Traits, _Alloc>&&,_Elem const* const)
std::operator+(basic_string<_Elem, _Traits, _Alloc>&&,_Elem const)
std::operator==(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator==(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator==(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator!=(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator!=(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator!=(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator<(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator<(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator<(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator>(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator>(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator>(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator<=(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator<=(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator<=(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator>=(basic_string<_Elem, _Traits, _Alloc> const&,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator>=(_Elem const* const,basic_string<_Elem, _Traits, _Alloc> const&)
std::operator>=(basic_string<_Elem, _Traits, _Alloc> const&,_Elem const* const)
std::operator>>(basic_istream<_Elem, _Traits>&,basic_string<_Elem, _Traits, _Alloc>&)
std::operator<<(basic_ostream<_Elem, _Traits>&,basic_string<_Elem, _Traits, _Alloc> const&)
std::literals::string_literals::operator""s(char const*,size_t)
std::literals::string_literals::operator""s(wchar_t const*,size_t)
std::literals::string_literals::operator""s(char16_t const*,size_t)
std::literals::string_literals::operator""s(char32_t const*,size_t)

Before this commit the output would be simply:

Dummy(std::string)

The dummy function isn't even showing.
This must be a bug!
clang version: 15.0.2

foonathan commented

I can't reproduce that using clang 14.0.6 and do not have access to clang 15 at the moment. Can you verify that the issue does not appear when you downgrade to clang 14?

deadlocklogic commented

Ok, I will try with clang14 and get back to you as soon as possible.

deadlocklogic commented

Sorry for the late reply, looks like this issue is occurring with newer versions of clang.
I don't know if this should be closed, or needs more feedback but for sure it is breaking with newer versions that I had to revert this commit in my build.

foonathan commented

I have the test.h file you've shown above.

I then use ./cppast -v test.h -std=c++20 and get the expected output:

AST for 'test.h':
|-string (include directive): `#include <string>`
+-Dummy (function) [definition]: `static void Dummy(std::str
ing value);`

My clang version is:

clang version 15.0.7
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Does that fail for you? What is your clang version?

deadlocklogic commented

I am using clang version: 15.0.2, i will try with 15.0.7 and see what happens.

deadlocklogic commented

I will be trying LLVM 16.0.0 from chocolatey and see what happens.

deadlocklogic commented

Sorry for late reply, because usually like most developers I tend to be careful when updating libraries on my main machine.
And my caution was unnecessary because this step even broke the tests of cppast 🤦‍♂️.
Clang: version 16.0.2


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cppast_test.exe is a Catch v2.13.9 host application.
Run with -? for options

-------------------------------------------------------------------------------
cpp_class
-------------------------------------------------------------------------------
C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_class.cpp(10)
...............................................................................

C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_class.cpp(298): FAILED:
  REQUIRE( equal_types(idx, base.type(), *cpp_user_defined_type::build( cpp_type_ref(cpp_entity_id(""), "ns::base"))) )
with expansion:
  false

-------------------------------------------------------------------------------
cpp_type_alias
-------------------------------------------------------------------------------
C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_type_alias.cpp(193)
...............................................................................

C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_type_alias.cpp(514): FAILED:
  CHECK( equal_types(idx, alias.underlying_type(), *type) )
with expansion:
  false

-------------------------------------------------------------------------------
cpp_type_alias
-------------------------------------------------------------------------------
C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_type_alias.cpp(193)
...............................................................................

C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_type_alias.cpp(514): FAILED:
  CHECK( equal_types(idx, alias.underlying_type(), *type) )
with expansion:
  false

-------------------------------------------------------------------------------
cpp_variable
-------------------------------------------------------------------------------
C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_variable.cpp(13)
...............................................................................

C:\Users\user\Desktop\cppastTest\CppAstTest\thirdparty\cppast\test\cpp_variable.cpp(78): FAILED:
  REQUIRE( equal_types(idx, var.type(), type) )
with expansion:
  false

[simple file parser] [info] parsing file 'a.cpp'
[simple file parser] [info] parsing file 'b.cpp'
[simple file parser] [info] parsing file 'c.cpp'
===============================================================================
test cases:   44 |   41 passed | 3 failed
assertions: 2333 | 2329 passed | 4 failed

Apparently my main issue was solved thought.
So I will try with 15.0.7 and see what happens. (Still this what you will face when upgrading to version 16.0.2)

deadlocklogic commented

Ok version 15.0.7 works fine 👍.
Feel free to close this issue but remember version 16.0.2 will break the tests.
Thanks.