r205037 - Use constexpr again, this time portably

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Apr 10 18:14:05 PDT 2014


On Apr 10, 2014, at 17:43, Reid Kleckner <rnk at google.com> wrote:

> On Thu, Apr 10, 2014 at 5:35 PM, Nico Weber <thakis at chromium.org> wrote:
> On Thu, Apr 10, 2014 at 4:55 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Fri, Mar 28, 2014 at 12:27 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> Author: dexonsmith
> Date: Fri Mar 28 14:27:37 2014
> New Revision: 205037
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=205037&view=rev
> Log:
> Use constexpr again, this time portably
> 
> Responding to Justin's review of r205025.
> 
> Modified:
>     cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenPGO.cpp?rev=205037&r1=205036&r2=205037&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenPGO.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenPGO.cpp Fri Mar 28 14:27:37 2014
> @@ -815,8 +815,8 @@ namespace {
>  }
> 
>  static void emitRuntimeHook(CodeGenModule &CGM) {
> -  const char *RuntimeVarName = "__llvm_profile_runtime";
> -  const char *RuntimeUserName = "__llvm_profile_runtime_user";
> +  LLVM_CONSTEXPR const char *RuntimeVarName = "__llvm_profile_runtime";
> +  LLVM_CONSTEXPR const char *RuntimeUserName = "__llvm_profile_runtime_user";
> 
> These aren't even globals, they're just locals.  Why bother with constexpr at all?

 1. Setting these as constexpr tells someone reading the code up front
    that these variables are compile-time constants.  Personally I
    value that sort of information, so I const when I can.  Whether the
    verbosity is valuable here is debatable.

 2. It also provides a handy compile error if someone modifying the
    code that follows tries to assign to them.  Again, whether that's
    valuable here is debatable.

I went with Richard's suggestion to switch to *const, which mostly
accomplishes the former and still accomplishes the latter, but the
function is short enough I hardly have an opinion on this at all.



More information about the cfe-commits mailing list