Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors with "circular" includes #150

Open
Reen opened this issue Nov 7, 2022 · 14 comments
Open

Errors with "circular" includes #150

Reen opened this issue Nov 7, 2022 · 14 comments

Comments

@Reen
Copy link

Reen commented Nov 7, 2022

  • 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
Copy link
Collaborator

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

@Reen
Copy link
Author

Reen commented Nov 9, 2022

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 added a commit that referenced this issue Nov 13, 2022
@foonathan
Copy link
Collaborator

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
Copy link
Author

Reen commented Nov 18, 2022

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 foonathan reopened this Nov 22, 2022
@foonathan
Copy link
Collaborator

Okay, now I should have fixed it :D

@deadlocklogic
Copy link

deadlocklogic commented Dec 9, 2022

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
Copy link
Collaborator

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?

@foonathan foonathan reopened this Dec 9, 2022
@deadlocklogic
Copy link

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

@deadlocklogic
Copy link

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
Copy link
Collaborator

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
Copy link

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

@deadlocklogic
Copy link

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

@deadlocklogic
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants