Opinions on std::array?

[Greg Thompson]

Hi gang. In a recent CL where I was using a bog-standard C array for some constants. One reviewer asked (paraphrasing) "why aren't you using std::array here?" So I switched to that. Then another reviewer asked (paraphrasing) "what does std::array buy you here?", which I interpreted as "why are you using std::array here?"

I don't really see how std::array is a good thing, to be honest. It seems downright arcane to have to explicitly stuff the size of the array in the type name (maybe std::to_array in C++20 addresses this?). std::begin and std::end work just fine for a C array, so why use std::array? Is there some benefit I'm not seeing?

Thanks.

[Patrick Monette]

I like them. To me, the real benefits are:

- value sematics
- built-in lexicographical comparisons

- no array decay

If you're not going be using any of those features, I think a c-style array is more readable. This is especially true when doing multidimensional arrays.

[Peter Kasting]

On Thu, Aug 22, 2019 at 10:26 PM Greg Thompson <g...@chromium.org> wrote:

I don't really see how std::array is a good thing, to be honest. It seems downright arcane to have to explicitly stuff the size of the array in the type name (maybe std::to_array in C++20 addresses this?). std::begin and std::end work just fine for a C array, so why use std::array? Is there some benefit I'm not seeing?

The big one is:

* Prevents decay into a pointer, which might save you against bugs

Smaller wins:

* Blocks derived-to-base conversions, which similarly might save you against bugs

* Can be passed by value into a function (though since the function has to specify the array size this generally means using templates)

* Allows zero-length arrays

* Allows bounds-checked accesses via at()

* If your callee code assumed container.begin()/end() work instead of paranoidly using std::begin/end(container), things will still work

None of these are huge.

std::to_array() + auto definitely makes using std::arrays nicer; I would argue in C++20 std::array will generally be preferable over C arrays.

PK

[Jan van Oort]

Bounds-checking on access are imho a nice win and can avoid quite some bugs.

Jan van Oort
Head of Research
KIVU Technologies GmbH 
www.kivu.tech

From: Greg Thompson Sent: Freitag, 23. August 2019 07:27 To: Chromium-dev Reply To: g...@chromium.org Subject: [chromium-dev] Opinions on std::array?

Hi gang. In a recent CL where I was using a bog-standard C array for some constants. One reviewer asked (paraphrasing) "why aren't you using std::array here?" So I switched to that. Then another reviewer asked (paraphrasing) "what does std::array buy you here?", which I interpreted as "why are you using std::array here?"

I don't really see how std::array is a good thing, to be honest. It seems downright arcane to have to explicitly stuff the size of the array in the type name (maybe std::to_array in C++20 addresses this?). std::begin and std::end work just fine for a C array, so why use std::array? Is there some benefit I'm not seeing?

Thanks.

--
--
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...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKQnr0SbQWZsf7nvKRNBKH%2BeN6xXsZRZc0d9YgyPBge04PeGMw%40mail.gmail.com.

[Greg Thompson]

Thanks for the feedback. These are great points.

Regarding .at(), though, that's not an option for Chromium (or Google) development since it throws exceptions.

[Hans Wennborg]

From a code complexity standpoint (both for human readers and for the
compiler), a regular array is a lot simpler and for that reason I
think it's preferable in general.

Of course if you need to do things that std::array has built-in, like
copying the array to pass it by value, maybe the complexity trade-off
starts to swing towards std::array's advantage.

[Chris Palmer]

I'd rather have people create and use a `base::array` that does `CHECK` in `operator[]` and `at` (making them equivalent). See the base/containers — we've been adding basic sanity checks to them for a while (and I've got a project to add similar, compile-time-optional, checks to Abseil and libc++).

[Alan Cutter]

On Friday, 23 August 2019 18:22:52 UTC+10, Greg Thompson wrote:

Thanks for the feedback. These are great points.

Regarding .at(), though, that's not an option for Chromium (or Google) development since it throws exceptions.

.at() is fine to use, it crashes instead of throwing.

 

On Fri, Aug 23, 2019 at 8:44 AM Jan van Oort <j...@kivu.tech> wrote:

Bounds-checking on access are imho a nice win and can avoid quite some bugs.

Jan van Oort
Head of Research
KIVU Technologies GmbH 
www.kivu.tech

From: Greg Thompson Sent: Freitag, 23. August 2019 07:27 To: Chromium-dev Reply To: g...@chromium.org Subject: [chromium-dev] Opinions on std::array?

Hi gang. In a recent CL where I was using a bog-standard C array for some constants. One reviewer asked (paraphrasing) "why aren't you using std::array here?" So I switched to that. Then another reviewer asked (paraphrasing) "what does std::array buy you here?", which I interpreted as "why are you using std::array here?"

I don't really see how std::array is a good thing, to be honest. It seems downright arcane to have to explicitly stuff the size of the array in the type name (maybe std::to_array in C++20 addresses this?). std::begin and std::end work just fine for a C array, so why use std::array? Is there some benefit I'm not seeing?

Thanks.

--
--
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.

[Greg Thompson]

On Mon, Aug 26, 2019 at 2:07 AM Alan Cutter <alanc...@chromium.org> wrote:

On Friday, 23 August 2019 18:22:52 UTC+10, Greg Thompson wrote:

Thanks for the feedback. These are great points.

Regarding .at(), though, that's not an option for Chromium (or Google) development since it throws exceptions.

.at() is fine to use, it crashes instead of throwing.

Interesting. I've been under the impression that at() should be avoided more or less on these grounds:

• at() is for use over operator[] when you want to handle OOB access
• it has extra overhead for this
• we don't use exceptions, so we should handle range validity explicitly and never use at()

Though I don't see it in the style guide just now, I swear that the common guidance was "don't use at()".

If crashes resulting from exceptions thrown by STL funcs is something we rely on, do we already have the pony that Chris wants in the STL itself (meaning there's no need for a base::array)?

 

On Fri, Aug 23, 2019 at 8:44 AM Jan van Oort <j...@kivu.tech> wrote:

Bounds-checking on access are imho a nice win and can avoid quite some bugs.

Jan van Oort
Head of Research
KIVU Technologies GmbH 
www.kivu.tech

From: Greg Thompson Sent: Freitag, 23. August 2019 07:27 To: Chromium-dev Reply To: g...@chromium.org Subject: [chromium-dev] Opinions on std::array?

Hi gang. In a recent CL where I was using a bog-standard C array for some constants. One reviewer asked (paraphrasing) "why aren't you using std::array here?" So I switched to that. Then another reviewer asked (paraphrasing) "what does std::array buy you here?", which I interpreted as "why are you using std::array here?"

I don't really see how std::array is a good thing, to be honest. It seems downright arcane to have to explicitly stuff the size of the array in the type name (maybe std::to_array in C++20 addresses this?). std::begin and std::end work just fine for a C array, so why use std::array? Is there some benefit I'm not seeing?

Thanks.

--
--
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...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKQnr0SbQWZsf7nvKRNBKH%2BeN6xXsZRZc0d9YgyPBge04PeGMw%40mail.gmail.com.

--
--
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...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/4ea3383c-96af-402f-99da-2bc797d1366b%40chromium.org.

[Chris Palmer]

No, because part of the pony that I want — the sparkly unicorn horn — is that `operator[]` should also check. If you do happen to use `std::array` and only ever use `at`, that'd definitely be a beautiful creature and I would love it. But I want to not have to check (and you want me not to check, and you want to not get Clusterfuzz reports).

That unicorn horn really sparkles. It's magic! :)

You received this message because you are subscribed to a topic in the Google Groups "Chromium-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/a/chromium.org/d/topic/chromium-dev/DcxltYiLPjQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKQnr0R9Tk%3DH1E-2%2BC4jm8n4pdWLEM1i61QGsc5w7E4qcB8Z1g%40mail.gmail.com.

[Greg Thompson]

Is it worth wrapping up this thread with a concise recommendation that we can all follow? For example, prefer .at() over operator[], especially when the index is not obviously correct (e.g., operator[] for a for loop with no subtlety is okay)?

[Peter Kasting]

On Mon, Aug 26, 2019 at 11:19 PM Greg Thompson <g...@chromium.org> wrote:

Is it worth wrapping up this thread with a concise recommendation that we can all follow? For example, prefer .at() over operator[], especially when the index is not obviously correct (e.g., operator[] for a for loop with no subtlety is okay)?

I find code with .at() less clear to read, for similar reasons as code which handles DCHECK failure: it's not really clear whether the author thinks that problems can happen, or is being paranoid, or what.  The most common thing I have found when asking authors about such code is that they had no idea at() and [] were even different.

Since at() is not idiomatic in Chrome today, I would really like to avoid introducing more usage of it.

PK

[Chris Palmer]

Since memory corruption is rampant in Chromium today, I would really like to avoid introducing more usage of it. ;)

+Adrian Taylor has projects to quantify and to illustrate the memory corruption problem, and +Daniel Cheng and I are going to work on raising the What To Do In Base? question in a structured way. Additionally I have a (currently slow-moving) project to bring compile-time-configurable sanity checks (in production) to Abseil and then hopefully libc++, and of course we've been adding such checks to base for a while now.

In the meantime, at is straightforwardly documented as checking, and operator[] straightforwardly documented as not checking. The confusion there seems easily resolved, and it's an easy way to avoid having your code become a data point in Security Team's charts and graphs. :) (See also optional::value vs. optional::operator*.)

[Reid Kleckner]

I think it's going to be harder to convince people to change the dialect of C++ that they write than it is to add some configurable asserts into libc++ that Chrome can set. Ultimately if the checks are valuable you'll want to harden operator*, operator[], etc, and then any effort spent migrating to .at() will be wasted. So, I think we should just get to work on this feature request.

--

--
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...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAOuvq23j2b%3DEGEzVrcw00Yq30%3Db4foHmOHzH-khCbjbE4a56PA%40mail.gmail.com.

[Peter Kasting]

On Tue, Aug 27, 2019 at 10:31 AM Chris Palmer <pal...@chromium.org> wrote:

+Adrian Taylor has projects to quantify and to illustrate the memory corruption problem, and +Daniel Cheng and I are going to work on raising the What To Do In Base? question in a structured way. Additionally I have a (currently slow-moving) project to bring compile-time-configurable sanity checks (in production) to Abseil and then hopefully libc++, and of course we've been adding such checks to base for a while now.

Sure, and if you can manage to transparently check without an unreasonable perf cost, that will be a useful solution, because it will avoid modifying the apparent intent of code people write.  (Whether you will ever be able to do that is another matter.  I wish you well.  But I think we'd be better served taking the highest-risk surface areas of our code and rewriting them in Rust.)

As-is, though, using at() is so rare that it sticks out in a similar way that doing this does:

void foo(int* x) {

  if (!x)

    return;

  ...

This implies to the reader that |x| can be null.  If |x| can't be null, this code often misleads other maintainers in the future into inserting unnecessary null checks all over their own code.  This is why we discourage "paranoid" checking -- it's not about the perf cost of it, it's about the readability and maintenance cost.  Handling DCHECK failure is discouraged for the same reason.

If I see someone using at(), I assume that either (a) they don't know what they're doing or (b) they have reason to believe that their indexes go out of bounds here.  When (b) is not true, the code is misleading.  You can argue that my assumptions are bad, but unless we systematically replace [] with .at() throughout the codebase, I don't think they're out of step with what we actually practice.  FWIW, I assume similar things when people explicitly use checked numerics, checked casts, etc. -- that they have special reason to believe there is danger in that area that isn't commonly present.

(See also optional::value vs. optional::operator*.)

This is actually a great illustrative example.  I had no idea that one of those checked and the other didn't -- I thought they were identical.  And I'm one of the team style owners and probably more C++-knowledgeable than the median Chrome engineer.

It would be nice if all of us could memorize the whole language spec.  Unfortunately, C++ is huge, and we are fallible.

PK 

[Chris Palmer]

I'm on it. 

[Chris Palmer]

On Tue, Aug 27, 2019 at 10:43 AM Peter Kasting <pkas...@chromium.org> wrote:

Sure, and if you can manage to transparently check without an unreasonable perf cost, that will be a useful solution, because it will avoid modifying the apparent intent of code people write.  (Whether you will ever be able to do that is another matter.  I wish you well.  But I think we'd be better served taking the highest-risk surface areas of our code and rewriting them in Rust.)

Good news! You are not alone in this belief. :)

If I see someone using at(), I assume that either (a) they don't know what they're doing or (b) they have reason to believe that their indexes go out of bounds here.  When (b) is not true, the code is misleading.  You can argue that my assumptions are bad, but unless we systematically replace [] with .at() throughout the codebase, I don't think they're out of step with what we actually practice.  FWIW, I assume similar things when people explicitly use checked numerics, checked casts, etc. -- that they have special reason to believe there is danger in that area that isn't commonly present.

I usually find that people were not using checked numerics because they didn't know how bonkers C/C++ integer semantics really are — not because they already thought they had checked enough.

Basically, C/C++'s default semantics, across the board, are backwards. Hence Rust and other things.

(See also optional::value vs. optional::operator*.)

This is actually a great illustrative example.  I had no idea that one of those checked and the other didn't -- I thought they were identical.  And I'm one of the team style owners and probably more C++-knowledgeable than the median Chrome engineer.

It would be nice if all of us could memorize the whole language spec.  Unfortunately, C++ is huge, and we are fallible.

Even just enumerating C++'s undefined behaviors is an open project. I find that enjoyable*.

Anyway, we all agree that we need to make the language-as-used-by-Chromium-developers safe enough for Chromium's users, and Security Team is chugging along on that. Expect more in this space...

* "Enjoyable" means "bad" when a security person says it

[Darin Fisher]

On Wed, Aug 28, 2019, 3:55 AM Chris Palmer <pal...@chromium.org> wrote:

On Tue, Aug 27, 2019 at 10:43 AM Peter Kasting <pkas...@chromium.org> wrote:

Sure, and if you can manage to transparently check without an unreasonable perf cost, that will be a useful solution, because it will avoid modifying the apparent intent of code people write.  (Whether you will ever be able to do that is another matter.  I wish you well.  But I think we'd be better served taking the highest-risk surface areas of our code and rewriting them in Rust.)

Good news! You are not alone in this belief. :)

If I see someone using at(), I assume that either (a) they don't know what they're doing or (b) they have reason to believe that their indexes go out of bounds here. 

Hmm, I use at() pretty much only when I have say a pointer to a std::vector. Am I weird?

-Darin

When (b) is not true, the code is misleading.  You can argue that my assumptions are bad, but unless we systematically replace [] with .at() throughout the codebase, I don't think they're out of step with what we actually practice.  FWIW, I assume similar things when people explicitly use checked numerics, checked casts, etc. -- that they have special reason to believe there is danger in that area that isn't commonly present.

I usually find that people were not using checked numerics because they didn't know how bonkers C/C++ integer semantics really are — not because they already thought they had checked enough.

Basically, C/C++'s default semantics, across the board, are backwards. Hence Rust and other things.

(See also optional::value vs. optional::operator*.)

This is actually a great illustrative example.  I had no idea that one of those checked and the other didn't -- I thought they were identical.  And I'm one of the team style owners and probably more C++-knowledgeable than the median Chrome engineer.

It would be nice if all of us could memorize the whole language spec.  Unfortunately, C++ is huge, and we are fallible.

Even just enumerating C++'s undefined behaviors is an open project. I find that enjoyable*.

Anyway, we all agree that we need to make the language-as-used-by-Chromium-developers safe enough for Chromium's users, and Security Team is chugging along on that. Expect more in this space...

* "Enjoyable" means "bad" when a security person says it

--

--
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...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAOuvq20zHDa1qFKsA1KFmrZT1V11vTZbTGpbhpwHHbGHqhv%3D9Q%40mail.gmail.com.

[Scott Violet]

No. In fact I wouldn't even use at() with a pointer, I favor (*vector)[i].

  -Scott

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAP0-Qpt-CFon996Rbqi7H6Ur6m6Cm7hhO25TFAF2VF9d1hW-fA%40mail.gmail.com.

[Darin Fisher]

Yeah, I see that form a lot too. I'm just saying the only time I use at() is in these situations -- at() seems like less operator verbosity. Otherwise, I pretty much never use at().

-Darin

[Bruce Dawson]

> `operator[]` should also check

Are there plans to make the checking less verbose in this scenario? Our CHECK macros generate a lot of code which makes them expensive if they are in a hot path or left in too many places, if only because of the i-cache pressure that they cause. Range checking of operator[] can be as simple as a single compare and a conditional branch to an out-of-line "int 3" instruction. Checks everywhere will be much more palatable if that is the cost.

In the example below a simple "if (index >= size) __debugbreak();" consumes six bytes of code space (from *76 to *7C), whereas the CHECK macro consumes 40 bytes of code space (from *85 to *AD), plus some string constants, plus it somehow triggers the compiler to generate __security_cookie related code. Thus main() goes from 21 bytes to 90 bytes if we use CHECK instead of a simpler check, still not including the string constant space.

Greater memory safety would be great - I sometimes end up tracking down memory corruption crashes and they are a pain - but it will be easier to make pervasive if it is cheap.

int main(int argc, char** argv) {

01EE2F70  push        ebp  

01EE2F71  mov         ebp,esp  

01EE2F73  mov         eax,dword ptr [ebp+8]  

  if ((unsigned)argc >= ARRAYSIZE(mydata))

01EE2F76  cmp         eax,4  

01EE2F79  jb          main+0Ch (01EE2F7Ch)  

    __debugbreak();

01EE2F7B  int         3  

  return mydata[argc];

01EE2F7C  mov         eax,dword ptr mydata (020976C8h)[eax*4]  

01EE2F83  pop         ebp  

01EE2F84  ret  

int main(int argc, char** argv) {

01A12F70  push        ebp  

01A12F71  mov         ebp,esp  

01A12F73  push        edi  

01A12F74  push        esi  

01A12F75  sub         esp,0A4h  

01A12F7B  mov         eax,dword ptr [__security_cookie (01BB36B0h)]  

01A12F80  mov         edi,dword ptr [argc]  

01A12F83  xor         eax,ebp  

  CHECK((unsigned)argc < ARRAYSIZE(mydata));

01A12F85  cmp         edi,4  

01A12F88  mov         dword ptr [ebp-0Ch],eax  

01A12F8B  jb          main+3Dh (01A12FADh)  

01A12F8D  lea         esi,[ebp-0ACh]  

01A12F93  mov         ecx,esi  

01A12F95  push        offset string "(unsigned)argc < ARRAYSIZE(mydat"... (01B99428h)  

01A12F9A  push        0Dh  

01A12F9C  push        offset string "../../base/test/run_all_base_uni"... (01B9944Bh)  

01A12FA1  call        logging::LogMessage::LogMessage (017D1150h)  

01A12FA6  mov         ecx,esi  

01A12FA8  call        logging::LogMessage::~LogMessage (017D1240h)  

  return mydata[argc];

01A12FAD  mov         esi,dword ptr mydata (01BC86C8h)[edi*4]  

01A12FB4  mov         ecx,dword ptr [ebp-0Ch]  

01A12FB7  xor         ecx,ebp  

01A12FB9  call        __security_check_cookie (01A1309Fh)  

01A12FBE  mov         eax,esi  

01A12FC0  add         esp,0A4h  

01A12FC6  pop         esi  

01A12FC7  pop         edi  

01A12FC8  pop         ebp  

01A12FC9  ret  

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/CAOuvq20zHDa1qFKsA1KFmrZT1V11vTZbTGpbhpwHHbGHqhv%3D9Q%40mail.gmail.com.

--
--
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.

[Chris Palmer]

Yes, object code size is certainly a concern to us. Smallifying* CHECK is something that +Daniel Cheng has been working on (some results have already landed, I believe).

* Commonwealth folks: This is the US way of saying "ensmallulate".

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAOuvq20zHDa1qFKsA1KFmrZT1V11vTZbTGpbhpwHHbGHqhv%3D9Q%40mail.gmail.com.

--
--
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...@chromium.org.

[Daniel Cheng]

On Tue, Aug 27, 2019 at 10:43 AM Peter Kasting <pkas...@chromium.org> wrote:

On Tue, Aug 27, 2019 at 10:31 AM Chris Palmer <pal...@chromium.org> wrote:

+Adrian Taylor has projects to quantify and to illustrate the memory corruption problem, and +Daniel Cheng and I are going to work on raising the What To Do In Base? question in a structured way. Additionally I have a (currently slow-moving) project to bring compile-time-configurable sanity checks (in production) to Abseil and then hopefully libc++, and of course we've been adding such checks to base for a while now.

Sure, and if you can manage to transparently check without an unreasonable perf cost, that will be a useful solution, because it will avoid modifying the apparent intent of code people write.  (Whether you will ever be able to do that is another matter.  I wish you well.  But I think we'd be better served taking the highest-risk surface areas of our code and rewriting them in Rust.)

There are investigations into using Rust, but I suspect that simply moving the highest-risk parts to Rust isn't sufficient, unless we move the entire attack surface exposed from the browser to IPC to Rust. That would be a fairly major endeavour.

 

As-is, though, using at() is so rare that it sticks out in a similar way that doing this does:

void foo(int* x) {

  if (!x)

    return;

  ...

This implies to the reader that |x| can be null.  If |x| can't be null, this code often misleads other maintainers in the future into inserting unnecessary null checks all over their own code.  This is why we discourage "paranoid" checking -- it's not about the perf cost of it, it's about the readability and maintenance cost.  Handling DCHECK failure is discouraged for the same reason.

If I see someone using at(), I assume that either (a) they don't know what they're doing or (b) they have reason to believe that their indexes go out of bounds here.  When (b) is not true, the code is misleading.  You can argue that my assumptions are bad, but unless we systematically replace [] with .at() throughout the codebase, I don't think they're out of step with what we actually practice.  FWIW, I assume similar things when people explicitly use checked numerics, checked casts, etc. -- that they have special reason to believe there is danger in that area that isn't commonly present.

Rather than think of it as "I'm not sure the index is out of bounds", another way to think of it is imparting a memory-safe language's semantics: rather, by seeing the unconditional array index, the developer is *asserting* that the index is never out of bounds (since they would have explicitly checked otherwise).

Were you seeing this in an official build? Based on the fact that we're invoking the LogMessage destructor, I assume this is a non-official build.

It's true that CHECK() has some unavoidable overhead, but it shouldn't be 40 bytes: the trap sequence is currently 3 bytes on Windows: this doesn't count the cost of the conditional + jump, but I would expect that to be < 37 bytes :)

As Chris Palmer mentions, there is room for improvement. Without any LLVM tweaks, we can get this to 2 bytes on x86 platforms: however, we need compiler support to collapse it to just an int3. Doing either requires sorting out some metrics around stability though.

Daniel

[Daniel Cheng]

Confirmed with Bruce offline that it was because it was a non-official build. So while technically "working as intended", this is one of the most common points of confusion that I hear. So maybe we should make it so that only DCHECK_IS_ON() gives verbose CHECK() messages, and you otherwise get the compact CHECK() macro... not sure.

[Bruce Dawson]

For some reason I thought the CHECK overhead was the same on official builds, and I failed to test. CHECK overhead on non-official builds doesn't seem important so please ignore my size claims.