Re: [cmake-developers] Questions about coding conventions

[clinton]

- On Jun 14, 2016, at 8:09 AM, Daniel Pfeifer dan...@pfeifer-mail.de wrote:

> On Tue, Jun 14, 2016 at 3:14 PM, Brad King  wrote:
>> On 06/13/2016 10:16 AM, Brad King wrote:
 Can't `std::ifstream` and `std::ofstream` be used directly? It seams
 that kwsys does some workarounds
>>>
>>> Yes, std::{o,f}stream can be used directly.
>>
>> On second thought, std::{i,o}fstream should not be used to open files.
>> The cmsys::{i,o}fstream interfaces are not about compatibility, they
>> are about opening files on Windows using the wide character APIs by
>> converting from UTF-8 to UCS-2.
> 
> I see.
> 
> There are a few uses of std::{i,o}fstream. I guess we should migrate
> them all to kwsys.

Yes.  Thanks.
cmsys::{i,o}fstream is to support additional filenames on Windows by not using 
obsolete ANSI apis.

Clint
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

On Tue, Jun 14, 2016 at 3:14 PM, Brad King  wrote:
> On 06/13/2016 10:16 AM, Brad King wrote:
>>> Can't `std::ifstream` and `std::ofstream` be used directly? It seams
>>> that kwsys does some workarounds
>>
>> Yes, std::{o,f}stream can be used directly.
>
> On second thought, std::{i,o}fstream should not be used to open files.
> The cmsys::{i,o}fstream interfaces are not about compatibility, they
> are about opening files on Windows using the wide character APIs by
> converting from UTF-8 to UCS-2.

I see.

There are a few uses of std::{i,o}fstream. I guess we should migrate
them all to kwsys.
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Brad King]

On 06/13/2016 10:16 AM, Brad King wrote:
>> Can't `std::ifstream` and `std::ofstream` be used directly? It seams
>> that kwsys does some workarounds
> 
> Yes, std::{o,f}stream can be used directly.

On second thought, std::{i,o}fstream should not be used to open files.
The cmsys::{i,o}fstream interfaces are not about compatibility, they
are about opening files on Windows using the wide character APIs by
converting from UTF-8 to UCS-2.  Sorry for the confusion.

I've reverted the std-fstream topic for now.

-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Ben Boeckel]

On Mon, Jun 13, 2016 at 16:14:51 +0200, Daniel Pfeifer wrote:
> On Mon, Jun 13, 2016 at 2:09 PM, Ben Boeckel  wrote:
> > Usually NULL means "unset". See properties,
> > variable values, etc. As an output, any place which doesn't care should
> > already be using GetSafeDefinition().
> 
> I know that, at the moment, NULL is used for "unset" in many places. I
> am trying to figure out whether we could theoretically use "empy" to
> mean "unset".
> If there is a case where we distinguish between null and empty, this
> will not be possible.

And I'm saying that looking to port to GetSafeDefinition() will
highlight those that *can't* be converted easily which will get you use
cases for the current split.

> > As a concrete example, `set(CACHE)` cares about NULL versus *s == NULL:
> 
> Thanks for the example. I found the check for `s != NULL` in the
> second line. Can you help me finding the check for `*s == NULL`?

There isn't, hence the difference in behavior between NULL and empty.

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Brad King]

On 06/10/2016 05:43 PM, Daniel Pfeifer wrote:
>> Please look at documenting this in CONTRIBUTING.rst once we resolve
>> this discussion.
> 
> ok.

+1 to Steve's suggestion of putting it in cmake-developer(7).

>>> So far it is pretty consistent. But how to name free functions and
>>> macros? I have seen all kinds of variations.
>>
>> No convention was ever established for those.
> 
> I see. Any strong opinion whether a convention should be established?
> I would assume UPPER_CASE for macros. Any prefix?
> Any opinions about free functions?

IIRC several existing macros use the CMAKE_ prefix and are named
all upper-case.  I think that or CM_ would be fine.

For free functions I think naming them as cmCamelCase just like classes
has been done in a few places.

> So the convention would be to use `std::string` or
> `cmOptional` as return values.

Yes, though I think we should wait for this change until other pending
cleanups are done.

> Should `std::string const&` be allowed as return value?

I like value semantics but IIRC we tried to replace some "getters"
that currently return std::string by reference with values and it
showed up in profiling for heavily-called getters.

> Arguments should be preferred as `const&` I assume.

Yes.

> Some other questions:
> 
> Can't `std::ifstream` and `std::ofstream` be used directly? It seams
> that kwsys does some workarounds

Yes, std::{o,f}stream can be used directly.  KWSys may have some old
workarounds that haven't been fully removed yet.

> I see that `#include ` is preferred over `#include
> `. Are there old compilers supported that prohibit the latter
> or was it just not changed yet?

The dashboard will tell us.  As Steve said I think these are legacy.

> cmStandardIncludes.h includes many standard headers. That is against
> the idea of include-what-you-use. Is there a particular requirement or
> was it just not cleaned up yet?

We need cmsys/Configure.h to be the first file in every C++ translation
unit.  Also cmStandardIncludes.h used to have workarounds for system
headers on old platforms.  These days we can probably clean this up,
especially with tools like include-what-you-use.

> Should we begin using newer language features optionally by providing
> macros like CM_OVERRIDE?

Let's start with just CM_OVERRIDE.  We have a lot of virtual methods
and overrides so that will be a nice cleanup.  It will also be a test
case for initial introduction of the infrastructure needed to optionally
use C++11 features.

Thanks,
-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

On Mon, Jun 13, 2016 at 2:09 PM, Ben Boeckel  wrote:
> On Mon, Jun 13, 2016 at 00:03:29 +0200, Daniel Pfeifer wrote:
>> Can you show an example? To be clear: We are looking for a function,
>> that has a code path for `str == NULL` and a *different* codepath for
>> `str[0] = '\0'`.
>
> As input to functions?

Either input or output. I am interested about both cases.

> Usually NULL means "unset". See properties,
> variable values, etc. As an output, any place which doesn't care should
> already be using GetSafeDefinition().

I know that, at the moment, NULL is used for "unset" in many places. I
am trying to figure out whether we could theoretically use "empy" to
mean "unset".
If there is a case where we distinguish between null and empty, this
will not be possible.

> As a concrete example, `set(CACHE)` cares about NULL versus *s == NULL:

Thanks for the example. I found the check for `s != NULL` in the
second line. Can you help me finding the check for `*s == NULL`?

> const char* existingValue = state->GetCacheEntryValue(variable);
> if (existingValue &&
> (state->GetCacheEntryType(variable) != cmState::UNINITIALIZED)) {
>   // if the set is trying to CACHE the value but the value
>   // is already in the cache and the type is not internal
>   // then leave now without setting any definitions in the cache
>   // or the makefile
>   if (cache && type != cmState::INTERNAL && !force) {
> return true;
>   }
> }
>
> // if it is meant to be in the cache then define it in the cache
> if (cache) {
>   this->Makefile->AddCacheDefinition(variable, value.c_str(), docstring,
>  type, force);
> } else {
>   // add the definition
>   this->Makefile->AddDefinition(variable, value.c_str());
> }
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Ben Boeckel]

On Mon, Jun 13, 2016 at 00:03:29 +0200, Daniel Pfeifer wrote:
> Can you show an example? To be clear: We are looking for a function,
> that has a code path for `str == NULL` and a *different* codepath for
> `str[0] = '\0'`.

As input to functions? Usually NULL means "unset". See properties,
variable values, etc. As an output, any place which doesn't care should
already be using GetSafeDefinition().

As a concrete example, `set(CACHE)` cares about NULL versus *s == NULL:

const char* existingValue = state->GetCacheEntryValue(variable);
if (existingValue &&
(state->GetCacheEntryType(variable) != cmState::UNINITIALIZED)) {
  // if the set is trying to CACHE the value but the value
  // is already in the cache and the type is not internal
  // then leave now without setting any definitions in the cache
  // or the makefile
  if (cache && type != cmState::INTERNAL && !force) {
return true;
  }
}

// if it is meant to be in the cache then define it in the cache
if (cache) {
  this->Makefile->AddCacheDefinition(variable, value.c_str(), docstring,
 type, force);
} else {
  // add the definition
  this->Makefile->AddDefinition(variable, value.c_str());
}

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

On Sun, Jun 12, 2016 at 4:26 PM, Ben Boeckel  wrote:
> On Fri, Jun 10, 2016 at 15:30:05 +0200, Daniel Pfeifer wrote:
>> Passing and returning strings: We have `const char*`, `std::string`,
>> and `std::string const&`. Unifying this can affect performance.
>> Storing `const char*` in a `std::string` creates a (potentially
>> unneded) copy (assuming it is not null).
>
> I went through and changed all `const char*` paramters to `std::string
> const&` where the pointer was given to a string within the function
> (unless it was an error path or such). This allows eliding another
> allocation.

I am aware of that. In case of passing `foo.c_str()`, this moved the
creation of the unnecessary copy from inside the function to the call
site, resulting in more allocations rather than less, assuming the
function is called more than once. It is important to run clang-tidy
after each such refactoring to remove the now unnecessary c_str()
calls.

>> `const char*` can distinguish
>> invalid from empty. Do we actually need this distinction?
>
> It's used all over the place.

Can you show an example? To be clear: We are looking for a function,
that has a code path for `str == NULL` and a *different* codepath for
`str[0] = '\0'`.
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Ben Boeckel]

On Fri, Jun 10, 2016 at 15:30:05 +0200, Daniel Pfeifer wrote:
> Passing and returning strings: We have `const char*`, `std::string`,
> and `std::string const&`. Unifying this can affect performance.
> Storing `const char*` in a `std::string` creates a (potentially
> unneded) copy (assuming it is not null).

I went through and changed all `const char*` paramters to `std::string
const&` where the pointer was given to a string within the function
(unless it was an error path or such). This allows eliding another
allocation.

> `const char*` can distinguish
> invalid from empty. Do we actually need this distinction?

It's used all over the place.

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

On Fri, Jun 10, 2016 at 3:43 PM, Brad King  wrote:
> On 06/10/2016 09:30 AM, Daniel Pfeifer wrote:
>> By looking at the CMake source code, there are some inconsistencies
>> regarding coding conventions. This is not a big problem and fixing
>> them probably does not have a high priority.
>> I would like to know what conventions to follow for new code.
>
> Please look at documenting this in CONTRIBUTING.rst once we resolve
> this discussion.

ok.

>> Formatting: No longer an issue. A .clang-format is provided and most
>> inconsistencies have disappeared.
>
> Yes, and thanks for all your work on that.
>
>> Braces around single conditional statements: This is currently not
>> consistent. Should they be avoided? Should they be required? We could
>> add the missing ones with clang-tidy. If we want them.
>
> The intention has been to use braces for all blocks even with single
> statements.  Due to lack of documentation and enforcement this has
> not been maintained consistently.

I have added some missing ones:
https://cmake.org/gitweb?p=stage/cmake.git;a=commit;h=a16bf141bc5d393b27d13d8235d95a1b034052c2

>> Naming conventions: Classes are named  cmLikeThis. Member functions
>> and member variables are named LikeThis. Local variables are named
>> likeThis. Members are always accessed with `this->`.
>
> Yes, though there are also many local variables named `like_this` too.
> I have no strong feelings on enforcing local variable name conventions.
>
>> So far it is pretty consistent. But how to name free functions and
>> macros? I have seen all kinds of variations.
>
> No convention was ever established for those.

I see. Any strong opinion whether a convention should be established?
I would assume UPPER_CASE for macros. Any prefix?
Any opinions about free functions?

>> `const T` or `T const` (also: `const T&` or `T const&`): Currently,
>> CMake uses both. I have not analyzed which one dominates.
>
> I prefer `T const` always, except for `const char*` specifically.

Good.

>> Passing and returning strings: We have `const char*`, `std::string`,
>> and `std::string const&`. Unifying this can affect performance.
>> Storing `const char*` in a `std::string` creates a (potentially
>> unneded) copy (assuming it is not null). `const char*` can distinguish
>> invalid from empty. Do we actually need this distinction?
>
> Last year we had a few cleanup passes done along those lines, but
> it is far from consistent everywhere.  There  are cases where we
> need to distinguish invalid from empty, like GetDefinition().  It
> would be nice to have an optional or something like that
> so we can keep the distinction but avoid using `const char*` all
> over.  Any change like this should be introduced gradually, as
> many of the related areas are performance sensitive.

So the convention would be to use `std::string` or
`cmOptional` as return values. Should `std::string
const&` be allowed as return value?
Arguments should be preferred as `const&` I assume.


Some other questions:

Can't `std::ifstream` and `std::ofstream` be used directly? It seams
that kwsys does some workarounds for Visual Studio 2005 and newer.
That surprises me. Workarounds are usually used for older, not newer
compilers.

I see that `#include ` is preferred over `#include
`. Are there old compilers supported that prohibit the latter
or was it just not changed yet?

cmStandardIncludes.h includes many standard headers. That is against
the idea of include-what-you-use. Is there a particular requirement or
was it just not cleaned up yet?

Should we begin using newer language features optionally by providing
macros like CM_OVERRIDE?
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

On Fri, Jun 10, 2016 at 5:19 PM, Tobias Hunger  wrote:
> On Fr, 2016-06-10 at 16:34 +0200, Daniel Pfeifer wrote:
>> If used consistently, it indicates that you are dealing with a member.
>> I personally prefer `this->` over `m_`. With semantic syntax
>> highlighting you probably don't need either of them. But then again,
>> you often look at code that does not have semantic highlighting (eg.
>> inside diffs).
>
> So you optimize for teletype terminals and punish everybody that has invested 
> in
> new stuff like screens with *color* in the last couple of decades:-/
>
> /me is trapped in 1995!

In 2016, semantic highlighting is the exception rather than the norm.
I expect that to remain that way for some years to come.
Git diff output, compiler diagnostics, github, the woboc code browser
are all very colorful. Yet, members are not highlighted anywhere.
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Tobias Hunger]

On Fr, 2016-06-10 at 16:34 +0200, Daniel Pfeifer wrote:
> If used consistently, it indicates that you are dealing with a member.
> I personally prefer `this->` over `m_`. With semantic syntax
> highlighting you probably don't need either of them. But then again,
> you often look at code that does not have semantic highlighting (eg.
> inside diffs).

So you optimize for teletype terminals and punish everybody that has invested in
new stuff like screens with *color* in the last couple of decades:-/

/me is trapped in 1995!

Best Regards,
Tobias

-- 
Tobias Hunger, Senior Software Engineer | The Qt Company
The Qt Company GmbH, Rudower Chaussee 13, D-12489 Berlin
Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho. Sitz der
Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Brad King]

On 06/10/2016 10:34 AM, Daniel Pfeifer wrote:
>> May I asked why "this->" is used so often?
> 
> If used consistently, it indicates that you are dealing with a member.
> I personally prefer `this->` over `m_`. ...
> you often look at code that does not have semantic highlighting (eg.
> inside diffs).

Yes, and this has been part of our coding convention from the beginning
of CMake.

-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

On Fri, Jun 10, 2016 at 4:16 PM, Tobias Hunger  wrote:
> On Fr, 2016-06-10 at 15:30 +0200, Daniel Pfeifer wrote:
>> Naming conventions: Classes are named  cmLikeThis. Member functions
>> and member variables are named LikeThis. Local variables are named
>> likeThis. Members are always accessed with `this->`. So far it is
>> pretty consistent. But how to name free functions and macros? I have
>> seen all kinds of variations.
>
> May I asked why "this->" is used so often? I find it totally annoying to skip
> this line noise all the time and I never encountered this convention before.

If used consistently, it indicates that you are dealing with a member.
I personally prefer `this->` over `m_`. With semantic syntax
highlighting you probably don't need either of them. But then again,
you often look at code that does not have semantic highlighting (eg.
inside diffs).
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Tobias Hunger]

On Fr, 2016-06-10 at 15:30 +0200, Daniel Pfeifer wrote:
> Naming conventions: Classes are named  cmLikeThis. Member functions
> and member variables are named LikeThis. Local variables are named
> likeThis. Members are always accessed with `this->`. So far it is
> pretty consistent. But how to name free functions and macros? I have
> seen all kinds of variations.

May I asked why "this->" is used so often? I find it totally annoying to skip
this line noise all the time and I never encountered this convention before.

Best Regards,
Tobias

-- 
Tobias Hunger, Senior Software Engineer | The Qt Company
The Qt Company GmbH, Rudower Chaussee 13, D-12489 Berlin
Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho. Sitz der
Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] Questions about coding conventions

[Brad King]

On 06/10/2016 09:30 AM, Daniel Pfeifer wrote:
> By looking at the CMake source code, there are some inconsistencies
> regarding coding conventions. This is not a big problem and fixing
> them probably does not have a high priority.
> I would like to know what conventions to follow for new code.

Please look at documenting this in CONTRIBUTING.rst once we resolve
this discussion.

> Formatting: No longer an issue. A .clang-format is provided and most
> inconsistencies have disappeared.

Yes, and thanks for all your work on that.

> Braces around single conditional statements: This is currently not
> consistent. Should they be avoided? Should they be required? We could
> add the missing ones with clang-tidy. If we want them.

The intention has been to use braces for all blocks even with single
statements.  Due to lack of documentation and enforcement this has
not been maintained consistently.

> Naming conventions: Classes are named  cmLikeThis. Member functions
> and member variables are named LikeThis. Local variables are named
> likeThis. Members are always accessed with `this->`.

Yes, though there are also many local variables named `like_this` too.
I have no strong feelings on enforcing local variable name conventions.

> So far it is pretty consistent. But how to name free functions and
> macros? I have seen all kinds of variations.

No convention was ever established for those.

> `const T` or `T const` (also: `const T&` or `T const&`): Currently,
> CMake uses both. I have not analyzed which one dominates.

I prefer `T const` always, except for `const char*` specifically.

> Passing and returning strings: We have `const char*`, `std::string`,
> and `std::string const&`. Unifying this can affect performance.
> Storing `const char*` in a `std::string` creates a (potentially
> unneded) copy (assuming it is not null). `const char*` can distinguish
> invalid from empty. Do we actually need this distinction?

Last year we had a few cleanup passes done along those lines, but
it is far from consistent everywhere.  There  are cases where we
need to distinguish invalid from empty, like GetDefinition().  It
would be nice to have an optional or something like that
so we can keep the distinction but avoid using `const char*` all
over.  Any change like this should be introduced gradually, as
many of the related areas are performance sensitive.

-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

[cmake-developers] Questions about coding conventions

[Daniel Pfeifer]

Hi,

By looking at the CMake source code, there are some inconsistencies
regarding coding conventions. This is not a big problem and fixing
them probably does not have a high priority.
I would like to know what conventions to follow for new code.

Formatting: No longer an issue. A .clang-format is provided and most
inconsistencies have disappeared.

Braces around single conditional statements: This is currently not
consistent. Should they be avoided? Should they be required? We could
add the missing ones with clang-tidy. If we want them.

Naming conventions: Classes are named  cmLikeThis. Member functions
and member variables are named LikeThis. Local variables are named
likeThis. Members are always accessed with `this->`. So far it is
pretty consistent. But how to name free functions and macros? I have
seen all kinds of variations.

`const T` or `T const` (also: `const T&` or `T const&`): Currently,
CMake uses both. I have not analyzed which one dominates.

Passing and returning strings: We have `const char*`, `std::string`,
and `std::string const&`. Unifying this can affect performance.
Storing `const char*` in a `std::string` creates a (potentially
unneded) copy (assuming it is not null). `const char*` can distinguish
invalid from empty. Do we actually need this distinction? Or should we
assume empty==invalid and always prefer `std::string const&`
arguments? Or should we introduce a C++17-like cmStringView?

cheers, Daniel
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers