static constexpr char[] in headers

[Ilya Sherman]

In a couple of recent reviews* now, I've seen these patterns:

foo.h

---

class Foo {

 public:

  static constexpr char kSomeString[] = "Some string";

  static constexpr const char* kSomeOtherString = "Some other string";

  // etc.

};

foo.cc

---

constexpr char Foo:kSomeString[];

constexpr const char* Foo:kSomeOtherString;

// etc.

Is this pattern encouraged within Chromium?  To me, it looks really odd to inline the string value in the header, and makes me wonder whether the string is going to potentially be compiled in multiple times across different compilation units.  Am I just behind the times?

Also, am I right to think that we prefer the array-style type "constexpr char[]" over the pointer-style type "constexpr const char*"?

Thanks!

~ilya

*Most recent example: https://chromium-review.googlesource.com/c/chromium/src/+/719607/4/third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator.h#58

[Jamie Madill]

I can confirm that some compilers will instantiate two or more strings.

https://groups.google.com/forum/#!topic/angleproject/zC690uiOlDM

I think given that it might be a bad pattern.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAA3nRahb05GgWnat%2Bz9-pvn7d%3DmofRxL-NmrpaQz-0sTUDuZhQ%40mail.gmail.com.

[Peter Kasting]

On Sun, Oct 15, 2017 at 2:02 PM, Jamie Madill <jma...@chromium.org> wrote:

I can confirm that some compilers will instantiate two or more strings.

https://groups.google.com/forum/#!topic/angleproject/zC690uiOlDM

Are you certain this is parallel?  That doesn't look like the same kind of initialization (string literal inside an aggregate?).  I don't know that this instance sheds light on the root post here.

PK

[Yuta Kitamura]

On Sat, Oct 14, 2017 at 10:14 AM, Ilya Sherman <ishe...@chromium.org> wrote:

In a couple of recent reviews* now, I've seen these patterns:

foo.h

---

class Foo {

 public:

  static constexpr char kSomeString[] = "Some string";

  static constexpr const char* kSomeOtherString = "Some other string";

  // etc.

};

foo.cc

---

constexpr char Foo:kSomeString[];

constexpr const char* Foo:kSomeOtherString;

// etc.

C++-language-wise, both of these constants have external linkage in C++14, so I would expect the compilers generate just one canonical instance for each constant. But the compilers may also "inline" the value of the constexpr constants, so we may have two or more instances in the end. A rational compiler may want to inline only short strings, and use the defined value for others.

After C++17, the definition part won't be necessary as long as you declare a constant with constexpr. But this change probably has nothing to do with those compiler behavior.

I think we can just leave those optimizations to the compilers, unless we find an evidence that a specific compiler behaves in a problematic way...

 

Is this pattern encouraged within Chromium?  To me, it looks really odd to inline the string value in the header, and makes me wonder whether the string is going to potentially be compiled in multiple times across different compilation units.  Am I just behind the times?

Also, am I right to think that we prefer the array-style type "constexpr char[]" over the pointer-style type "constexpr const char*"?

The contents of the string will be generated in the resulted object in both cases. Additionally, in the latter case, an extra pointer value will also be generated (if I understand the compiler behavior correctly). So, if there's no other restrictions, the array-style should be better.

[Daniel Cheng]

My personal preference is for static constexpr char[] as well. It's a consistent and always safe way of declaring a truly constant string (whereas if one writes the traditional static const char* kStringConstant--note the missing const qualifier on the pointer value itself--kStringConstant is only mostly constant, as kStringConstant itself can still be assigned).

That being said, I've heard concerns that static constexpr char[] would prevent string pooling (e.g. https://chromium-review.googlesource.com/c/chromium/src/+/581908/4/components/domain_reliability/google_configs.cc#538). And in theory, the optimizer should figure out that extra pointer indirection isn't actually needed, so the resultant code would be identical (modulo string folding).

However, I feel like our toolchain should be able to handle this (and if it doesn't, we should fix it: identical code folding also breaks the 'every unique definition should have a unique address' assumption already).

Daniel

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFJcur9O1r%2Bx1byQX-GJ65i1MwdkU%3DrbfYyWoyCQ7gMKUFr8ZQ%40mail.gmail.com.

[bruce...@chromium.org]

I'd like to hijack this thread to discuss "static constexpr char foo[] =" in functions - please use it. Or, at least, prefer it over "static const std::string foo = ". I'm cleaning up a few instances of this (see crrev.com/c/708047) and it would be great if we could avoid adding them.

The local std::string variables that I'm cleaning up were only found because they were static and therefore caused atexit destructors to be generated. There are probably lots of non-static std::string constants out there as local variables and it would be nice to avoid their cost.