On Mon, Apr 01 2019, Rafael Sadowski <[email protected]> wrote:
> On Mon Apr 01, 2019 at 12:04:38PM +0200, Jeremie Courreges-Anglas wrote:
>> On Mon, Apr 01 2019, Rafael Sadowski <[email protected]> wrote:
>> > Enable clang extra tools in devel/llvm to provide some useful C/C++
>> > tools. I started with the extra tools because we don't have an C++ Language
>> > Server Protocol (LSP) server in the tree but I want to play with it.
>> >
>> > However, build and works fine on amd64. Notable changes:
>> >
>> > - Zap all ":Bool" tags in cmake configure args. It's 2019 and cmake is
>> >   smart enough.
>> 
>> I'm no cmake guru, what do the cmake authors/best practices say here?
>
> https://cmake.org/cmake/help/v3.6/manual/cmake.1.html See -D:
>
> "If the :<type> portion is given it must be one of the types specified
> by the set() command documentation for its CACHE signature."
>
> Okay this is useless in almost all our cases because what we do is to
> modify an option()[1] and NOT a set() with a type.
>
> For example, LLVM_ENABLE_RTTI is in llvm defined as:
>
>  option(LLVM_ENABLE_RTTI "Enable run time type information" OFF)
>
> and we can read the following in [1]:
>
> "Provide an option for the user to select as ON or OFF. If no initial
> value is provided, OFF is used."
>
> I see no type hint. So the best practices should be:
>
> Use ON and OFF without a type!
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> This is the reason why other stuff works also:
>
> From logical-expressions:
> $<BOOL:string>
>
>     Converts string to 0 or 1 according to the rules of the if()
>     command. Evaluates to 0 if any of the following is true:
>
>     - string is empty,
>     - string is a case-insensitive equal of 0, FALSE, OFF, N, NO, IGNORE, or 
> NOTFOUND, or
>     - string ends in the suffix -NOTFOUND (case-sensitive).
>
>     Otherwise evaluates to 1.
>
> 1: https://cmake.org/cmake/help/v3.0/command/option.html?highlight=option
> 2: 
> https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#logical-expressions

Hmmk so using just ON/OFF just looks cleaner.  Unless someone goes on
a rampage to unify all cmake CONFIGURE_ARGS in the tree, I'd prefer to
minimize churn and keep llvm consistent.

>> 
>> > - Set CLANG_ENABLE_STATIC_ANALYZER=True, CLANG_INCLUDE_TESTS=True to
>> >   build clang-tidy and enable test. (It is ignored if the folder extra
>> >   don't exists)
>> > - Adjust WANTLIB
>> >
>> > Comments? OK?
>> 
>> In this port I'm caring mostly about what is actually used by the ports
>> tree.  So if the extras subpackage breaks somehow, expect me to disable
>> it, not to fix it.
>
> I'll take care of -extras

We still need to ensure that -extras builds reliably, and that it builds
on non-amd64.  That's the main reason why I prefer to postpone this
after 6.5.

>> 
>> You say you "want to play with it".  Do you have an actual use case?
>> I think it's too late to add "extra" stuff in this release cycle.
>
> Clang-tidy and clangd worked-out-of box with qt-creator and a camke+c/c++
> project.
>
> My future use-case is clangd+vim = c++ autocomplete. But qt-creator
> proves to me that it works, now.
>
>> 
>> Comments inline,
>> 
>
> New diff with Stuart'S notes. Thanks Stuart.

Stuart also had valid points about DESCR and PLIST. ;)

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to