conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
7.98k stars 953 forks source link

[question] cppstd helper utilities #6687

Open fulara opened 4 years ago

fulara commented 4 years ago

As per contribution guide I am first asking question, only then drafting a pull request.

I was thinking of slightly reworking functionality in tools/settings.py - this file has functionalities around cppstd.

What I would see beneficial:

1) Add mirroring function for 'max' cppstd. 2) Rework code so that deduced_cppstd is public 3) Add method like.. deduced_cppstd_long which returns cppstd in form of '1998', '2011', so that comparision in recipes is easier for exampel: if cppstd > 11 is now tricky because you have to keep mind of '98' and also you have to remember that cppstd defaults to None if not specified in the profile.

What do you guys think? Can I go ahead and create a pr for this?

Minimonium commented 4 years ago

Hi, Fulara!

First of all please read the issue here on general guidelines of how to handle the standard version in recipes: https://github.com/conan-io/conan-center-index/issues/54

About your points - we'd appreciate it if you'd provide as with use cases for exposing that functionality.

fulara commented 4 years ago

Hello @Minimonium

Thanks, i've given it a read, but there are few problems that are not answered anywhere afaik. if projects has a minimum cppstd, thats easy - you just throw exception iftried to compiled with something else, :) However currently conan does not expose afaik the way to give user 'default cppstd'. its just there and compiler.cppstd is empty. The real fun starts when your project does not compile with c++17 - (why, why why! did they have to delete the throw(Ex) rather than deprecating it in the compilers as they did with auto_ptr - thats my biggest pain )

So I wanted to add following functionalities because: 1) if we have utilities to check min_cppstd why we dont have utilities to check max_cppstd? Thats very common problem within my projects that they simply dont compile with c++17 and there is nothing we can do about them because they are closed source. so I am doing something like:

        if not self.settings.compiler.cppstd in ["98", "gnu98", "11", "gnu11", "14", "gnu14"]:
            raise Exception("{0} library only supports up to cpp14, as it uses throw(Exception)".format(self.name))

Is this bad? Not really pretty self contained, however I am living on assumption that compiler.cppstd is set in my world - otherwise I would be pretty much doomed when compiler with default cppstd == 17 comes in.

2) Afaik there is a lack of querying conan 'hey conan tell me what version of cppstd i am building with' Every recipe i saw checks if self.settings.cppstd is defined.. if it isnt - then it hopes for the best. Tools have that logic, the deduce_cppstd function but it seems to be hidden in internals.

3) there is no easy way to compare cppstd like cppstd > 11, because of the '98' .. thats why having a 'long' version wuold be nice. if(cppstd < 2017 ) { "we are good!" ). tools has that logic embeded but does not expose it at all, so clients have to write it themselves.

Minimonium commented 4 years ago

Thank you a lot for your explanation!

Yes, now I see the case for the uplifting of the deduced_cppstd and less_than functions into general tools. Also having helpers for check/valid_max_cppstd would be nice.

Further on I think that having a general validation function that would follow the https://github.com/conan-io/conan-center-index/issues/54 with the addition of upper bound would be very beneficial.

madebr commented 3 years ago

Does there exists a function to determine the maximum c++ standard a compiler supports already? e.g. a function tools.compiler_supports_cppstd(self.settings.compiler, 17) should return True because my compiler (=gcc9) supports c++17.

Minimonium commented 3 years ago

@madebr It was quite a lot of time since I looked into the matter, but I'm pretty sure that this is handled by Conan through cppstd filtering. There was a map of compatible cppstd for concrete compilers somewhere in the internals which didn't allow to set incompatible setting combinations at all. Could you confirm?

madebr commented 3 years ago

tools.valid_min_cppstd checks the default c++ standard = standard without c++ flag. I don't know about an option how to filter out compilers that do not support c++17, but allow compilers that have a lower default c++ standard but still support c++17.

fulara commented 3 years ago

Hello @madebr ,

When I was looking at this I thought the implmentation would be straightforward, thats why I said i'll do it ( which I have never done. )

Function deduce_cppstd returns the cppstd, and then you have to just to the greater_equal check. https://github.com/conan-io/conan/blob/a9a81ae83191afc87c3d103a792c9e729da0ae2a/conans/client/tools/settings.py#L60

Unfortunatelly all of those methods are hidden so they cant be used, I planned to rework that file.

edit: just noticed th emay be cloed by...

madebr commented 3 years ago

Is there a look-up table somewhere that says for each compiler version what the default c++ version + maximum supported c++ version is? e.g.:

{
    "gcc": {
        "9": {"default": 2011, "max": 2017,},
    },
    "Visual Studio": {
        "15": {"default": None, "max": 2017,},
        "16": {"default": None, "max": 2020,},
    }

A look-up table would proably become too messed up, but a function to access the (min, default) would be useful, if not to check/throw exceptions.

Minimonium commented 3 years ago

@madebr https://github.com/conan-io/conan/blob/develop/conans/client/build/cppstd_flags.py

The cppstd version supported by a given compiler shouldn't be the matter of recipes, but the Conan settings-handling itself.

madebr commented 3 years ago

How would you handle this then?

I have a recipe that requires c++17 and which interface (=headers do not expose any c++ symbols). I would like to filter out all compilers not supporting c++17.

fulara commented 3 years ago

reimplement https://github.com/conan-io/conan/blob/a9a81ae83191afc87c3d103a792c9e729da0ae2a/conans/client/tools/settings.py#L40 and:

        def add_millennium(_cppstd):
            return "19%s" % _cppstd if _cppstd == "98" else "20%s" % _cppstd

and do an if >= 2017.

:)

However I disagree with @Minimonium here, in my interpretation conan has not fully embraced the usage of cppstd. the conanfiles in conan-recipes often rely on the projects setting cppstd by themselves.. (ignoring compielr default. basically. - but respecting the cppstd indeed. )

Minimonium commented 3 years ago

Again. It should be impossible in principle to have incompatible (unsupported) cppstd with a compiler. You don't need to filter out anything, Conan already does that (or make a PR to do it better).

fulara commented 3 years ago

@Minimonium my original problem was around a situation that you have a codebase that onl y compiles with cppstd <=14. If you have a compiler that supports c++17 you need to filter out cppstd17. Maybe I misunderstood madebr's problem, but in my scenario you do need to filter out that cppstd.

I am spamming anyway, sorry! I wont be implementing this one so feel free to process with this how you want.

madebr commented 3 years ago

So you would just not provide a package for a package that will build perfectly on a compiler, but just not on the default c++ standard.

Minimonium commented 3 years ago

I'm confused myself and I think that there is a lot of confusion all around. 😀 I did the PR for cppstd logic a long time ago and I could be a little bit gloomy on details.

From what I recall right now - you never really need to know the maximum supported compiler version ever.

To support an upper bound in recipe checking all you need to have is the current deduced cppstd and the upper bound value. This function is an example of such check with bounds and even provides compatibility strategies (but these compatibility bits are defective since compatibility needs to be propagated downstream). https://github.com/conan-io/conan/blob/127f737a62200e7b979661cc81893a275800476c/conans/client/tools/settings.py#L271

The check for "is this cppstd version even supported by the current compiler" is redundant because Conan already checks that (I suppose there could be bugs if we have that discussion), you can't have cppstd=20 on gcc6, Conan will error on you.

@madebr I don't understand how the default C++ standard version is related. Please see the link to the function with upper bound that I linked.