0

I decided to use a bit of a heftier version of clippy to get myself ready for a version change. I was expecting some false positives but I'm not sure about this one. I got the following struct, and clippy tells me that every single function should be const. (clippy::missing_const_for_fn)

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ScheduleFields {
    years: Years,
    days_of_week: DaysOfWeek,
    months: Months,
    days_of_month: DaysOfMonth,
    hours: Hours,
    minutes: Minutes,
    seconds: Seconds,
}

impl ScheduleFields {
    // Constructor
    pub fn new(
        seconds: Seconds,
        minutes: Minutes,
        hours: Hours,
        days_of_month: DaysOfMonth,
        months: Months,
        days_of_week: DaysOfWeek,
        years: Years,
    ) -> ScheduleFields {
        ScheduleFields {
            years,
            days_of_week,
            months,
            days_of_month,
            hours,
            minutes,
            seconds,
        }
    }

    // Getters
    pub fn years(&self) -> &Years { &self.years }
    pub fn months(&self) -> &Months { &self.months }
    pub fn days_of_month(&self) -> &DaysOfMonth { &self.days_of_month }
    pub fn days_of_week(&self) -> &DaysOfWeek { &self.days_of_week }
    pub fn hours(&self) -> &Hours { &self.hours }
    pub fn minutes(&self) -> &Minutes { &self.minutes }
    pub fn seconds(&self) -> &Seconds { &self.seconds }
}
Typhaon
  • 828
  • 8
  • 27

1 Answers1

2

missing_const_for_fn will suggest flagging as const anything which can be flagged thus. Regardless of it being useful or not.

Here every single function can be const-ed, so clippy suggests doing that. And once you've marked them as const, clippy will find that most of their callers (which currently can't be const-ed because they call non-const functions) can also be const-ed.

Personally, I don't think this lint is useful or should be used: much like Copy, const is a very strong and restrictive API promise. It should only be enabled on a case-per-case basis after careful consideration.

Masklinn
  • 34,759
  • 3
  • 38
  • 57
  • I concur. You should give a second thought about constifying methods just because `clippy` says so. A method being `const` is part of the signature and if one day its implementation requires you to un-constify it, all downstream users of that method in a const-context are facing a semver-breaking change. – user2722968 Feb 25 '21 at 10:25
  • It's all in a private module though. If I make this const then only one other private method is flagged as able to be const-ed. What are the benefits/drawbacks of functions like these being const? – Typhaon Feb 25 '21 at 11:04
  • 1
    The benefits are that they can be used in a const context, the drawbacks is that you'll have to remove const if you change their implementation to use something which isn't const. Whether that has any relevance only you can say, I've no idea what the context is, and clippy has no way to perform value judgements. Neither did anyone here have any idea that it's "all in a private module", you just pasted a pub struct with a bunch of pub fns. Not that that changes anything: the module being private doesn't mean you can't re-export its content. – Masklinn Feb 25 '21 at 12:28