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

[question] Is compatibility_cppstd transitive #17657

Open
1 task done
tgurriet opened this issue Jan 29, 2025 · 6 comments · May be fixed by #17659
Open
1 task done

[question] Is compatibility_cppstd transitive #17657

tgurriet opened this issue Jan 29, 2025 · 6 comments · May be fixed by #17659
Assignees

Comments

@tgurriet
Copy link

tgurriet commented Jan 29, 2025

What is your question?

Hello,

I have a question regarding compatibility_cppstd.

If I set extension_properties = {"compatibility_cppstd": False} in the recipe of package A, does this mean that any package B that depends on A (either directly or indirectly) will be treated as if its compatibility_cppstd flag is set to False?

Thanks in advance!

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@AbrilRBS
Copy link
Member

Hi @tgurriet thanks a lot for your question.

This would not be the case, only the recipe marked as compatibility_cppstd: False will have its compatibility checks disabled. I'm adding a test in #17659 to clarify this behaviour, and will make sure that the wording in the docs also gets improved, thanks!

@tgurriet
Copy link
Author

Thanks for the fast answer. Shouldn't it be the default behavior though?

@AbrilRBS
Copy link
Member

AbrilRBS commented Jan 29, 2025

Not necessarily no, it would all dependend on the on the specific API that the consumer interacts with from its direct dependencies.

Note for example how the current approach does not seem to be an issue at scale in Conan Center Index, where cppstd missmatches are somewhat common.

If this behaviour is something you might need to change on your side for some reason or another, please do let us know so that we can discuss it :) there might be some ways to make this attribute transitive from within the compatibility plugin if you need (After testing, this might not be something doable from within the compatibility plugin itself)

@tgurriet
Copy link
Author

tgurriet commented Jan 29, 2025

The case I encounter today is related to the Eigen library and took me a second to understand.

structs with eigen data members need to have a particular alignment. Pre C++17, this is done by overriding the new and delete operator for the struct with a macro provided by eigen:

struct S
{
  Eigen::Vector4d v;

  EIGEN_MAKE_ALIGNED_OPERATOR_NEW
  // Expands to 
  //   void * operator new(std::size_t size)
  // {
  //   return Eigen::internal::conditional_aligned_malloc<true>(size);
  // }
 // ...
};

In C++17 and up, the macro is empty, and alignment is done via the new new operators.

So if you have a dependency that compiled a library pre C++17 that returns a pointer to S, and you then delete that pointer in your code compiled post C++17, you'll have a new/delete allocator mismatch (caught by sanitizers thankfully!).

@tgurriet
Copy link
Author

@AbrilRBS It feels like there should be a flag in global.conf to selected that compatibility behavior globally. Such a binary option is probably sufficient in practice.

But if there is such a thing as a per package control of ABI compatibility, this should be handled transitively IMO, as transitive package compatibility management is one of the core feature of conan.

@tgurriet
Copy link
Author

@AbrilRBS Regarding the test that was added, I believe it's downstream propagation that should be checked.

Package A has an ABI discontinuity from c++14 to 17. It defines extension_properties = {"compatibility_cppstd": False}.

Package B that depends on A will honor that requirement.

The question is, if package B does not define extension_properties = {"compatibility_cppstd": False}, will package C that depends on B fetch package A without worrying about this ABI discontinuity only declared in package A?

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

Successfully merging a pull request may close this issue.

2 participants