Replace CONFIG['CLANG_CXX'], CONFIG['GNU_CXX'] by CONFIG['CC_TYPE']

RESOLVED FIXED in Firefox 59

Status

enhancement
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: sylvestre, Assigned: sylvestre)

Tracking

Trunk
mozilla59

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

Assignee

Description

2 years ago
As suggested in bug 1391995 by Mike, we should use CONFIG['CC_TYPE'] instead of CONFIG['CLANG_CXX'], CONFIG['GNU_CXX'], etc

For the record, GNU_CXX is set when clang is used. This is why we have to use CONFIG['CC_TYPE'] in ('clang', 'gcc'):
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8902173 - Flags: review?(mh+mozilla)
Assignee

Comment 5

2 years ago
I had this error in try:

12:01:30     INFO -  mathematics.obj : error LNK2019: unresolved external symbol __tzcnt_u32 referenced in function _av_gcd
12:01:30     INFO -  mozavutil.dll : fatal error LNK1120: 1 unresolved externals

Not sure it is me...

Comment 6

2 years ago
mozreview-review
Comment on attachment 8902173 [details]
Bug 1394734 - Replace CONFIG['GNU_C*'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/173644/#review179884

I'm stopping at page 1, I guess there are more of the same problems all around.

Please do the changes in multiple patches, one variable at a time (you can group e.g. CLANG_CC/CLANG_CXX, though)

Also, on top of this, please remove the corresponding AC_SUBST and set_configs.

::: browser/app/moz.build:65
(Diff revision 3)
>      USE_LIBS += [ 'fuzzer' ]
>      LOCAL_INCLUDES += [
>          '/tools/fuzzing/libfuzzer',
>      ]
>  
> -if CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] == 'msvc':

Actually, subtly, _MSC_VER is set for clang-cl.

::: browser/app/moz.build:101
(Diff revision 3)
>  #
>  # The default heap size is 1MB on Win32.
>  # The heap will grow if need be.
>  #
>  # Set it to 256k.  See bug 127069.
> -if CONFIG['OS_ARCH'] == 'WINNT' and not CONFIG['GNU_CC']:
> +if CONFIG['OS_ARCH'] == 'WINNT' and CONFIG['CC_TYPE'] != 'gcc':

this should also exclude clang (which would mean mingw clang)

::: build/gecko_templates.mozbuild:33
(Diff revision 3)
>      if msvcrt == 'dynamic' or CONFIG['OS_ARCH'] != 'WINNT' or CONFIG['MOZ_ASAN']:
>          xpcomglue = 'xpcomglue'
>      elif msvcrt == 'static':
>          USE_STATIC_LIBS = True
>          xpcomglue = 'xpcomglue_staticruntime'
> -        if not CONFIG['GNU_CC']:
> +        if CONFIG['CC_TYPE'] != 'gcc':

same here

::: config/external/icu/defs.mozbuild:41
(Diff revision 3)
>  DISABLE_STL_WRAPPING = True
>  ALLOW_COMPILER_WARNINGS = True
>  
>  # We allow compiler warnings, but we can at least cut down on spammy
>  # warnings that get triggered for every file.
> -if CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'clang_cl':

dash, not underscore.

::: config/external/icu/defs.mozbuild:51
(Diff revision 3)
>      CXXFLAGS += [
>          '-Wno-macro-redefined',
>          '-Wno-microsoft-include',
>      ]
>  
> -if CONFIG['_MSC_VER'] and not CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'msvc' and CONFIG['CC_TYPE'] != 'clang_cl':

if == 'msvc' is true... != 'clang_cl' is obviously false...

::: dom/canvas/moz.build:225
(Diff revision 3)
>  LOCAL_INCLUDES += CONFIG['SKIA_INCLUDES']
>  
> -if CONFIG['GNU_CXX']:
> +if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
>      CXXFLAGS += ['-Wno-error=shadow']
>  
> -if CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] == 'msvc':

+ clang-cl

::: dom/media/moz.build:285
(Diff revision 3)
>      'DecoderTraits.cpp',
>  ]
>  
>  # Some codec-related code uses multi-character constants, which GCC and clang
>  # warn about. Suppress turning this warning into an error.
> -if CONFIG['GNU_CC'] or CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] in ('clang', 'clang_cl', 'gcc'):

!= 'msvc'

::: dom/media/moz.build:337
(Diff revision 3)
>      CXXFLAGS += [
>          '-Wno-error=attributes',
>          '-Wno-error=shadow',
>      ]
>  
> -if CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] == 'msvc':

+ clang-cl

::: dom/media/platforms/omx/moz.build:29
(Diff revision 3)
>  FINAL_LIBRARY = 'xul'
>  
> -if CONFIG['GNU_CXX']:
> +if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
>      CXXFLAGS += ['-Wno-error=shadow']
>  
> -if CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] == 'msvc':

+ clang-cl

::: dom/media/webrtc/moz.build:74
(Diff revision 3)
>  
>  # Suppress some GCC/clang warnings being treated as errors:
>  #  - about attributes on forward declarations for types that are already
>  #    defined, which complains about important MOZ_EXPORT attributes for
>  #    android API types
> -if CONFIG['GNU_CC'] or CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] in ('clang', 'clang_cl', 'gcc'):

!= 'msvc'
Attachment #8902173 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8904274 - Attachment is obsolete: true
Attachment #8904274 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8904273 - Attachment is obsolete: true
Attachment #8904273 - Flags: review?(mh+mozilla)

Comment 16

2 years ago
mozreview-review
Comment on attachment 8902173 [details]
Bug 1394734 - Replace CONFIG['GNU_C*'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/173644/#review183050

::: accessible/windows/ia2/moz.build:55
(Diff revision 5)
>  # The Windows MIDL code generator creates things like:
>  #
>  #   #endif !_MIDL_USE_GUIDDEF_
>  #
>  # which clang-cl complains about.  MSVC doesn't, so turn this warning off.
> -if CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'clang-cl':

not GNU_CC/GNU_CXX

::: config/external/ffi/moz.build:89
(Diff revision 5)
>          ffi_srcs = ('ffi.c', 'sysv.S', 'win32.S')
>      elif CONFIG['FFI_TARGET'] == 'X86_64':
>          ffi_srcs = ('ffi64.c', 'unix64.S', 'ffi.c', 'sysv.S')
>      elif CONFIG['FFI_TARGET'] == 'X86_WIN32':
>          # MinGW Build for 32 bit
> -        if CONFIG['CC_TYPE'] == 'gcc':
> +        if CONFIG['CC_TYPE'] in ('clang', 'gcc'):

If you really mean to change this, this should be a separate patch, as it's not a noop.

::: dom/media/moz.build:332
(Diff revision 5)
>  include('/ipc/chromium/chromium-config.mozbuild')
>  
>  # Suppress some GCC warnings being treated as errors:
>  #  - about attributes on forward declarations for types that are already
>  #    defined, which complains about an important MOZ_EXPORT for android::AString
> -if CONFIG['GNU_CC']:
> +if CONFIG['CC_TYPE'] in ('clang', 'gcc'):

There's another one in this file.

::: gfx/cairo/cairo/src/moz.build:196
(Diff revision 5)
>  
>  for var in ('MOZ_TREE_CAIRO', 'MOZ_TREE_PIXMAN'):
>      if CONFIG[var]:
>          DEFINES[var] = True
>  
> -if CONFIG['GNU_CC']:
> +if CONFIG['CC_TYPE'] in ('clang', 'gcc'):

There are others in this file.

::: media/libopus/moz.build:48
(Diff revision 5)
>                           'NetBSD', 'OpenBSD'):
>      DEFINES['HAVE_LRINTF'] = True
>  
>  if CONFIG['OS_ARCH'] == 'WINNT':
>      DEFINES['inline'] = '__inline'
> -    if CONFIG['GNU_CC']:
> +    if CONFIG['CC_TYPE'] in ('clang', 'gcc'):

There's another one in this file.

::: media/pocketsphinx/moz.build:54
(Diff revision 5)
>      'src/s2_semi_mgau.c',
>      'src/state_align_search.c',
>  ]
>  
>  # Suppress warnings in third-party code.
> -if CONFIG['GNU_CC']:
> +if CONFIG['CC_TYPE'] in ('clang', 'gcc'):

this file is gone.

::: security/pkix/warnings.mozbuild:1
(Diff revision 5)
> -if CONFIG['CLANG_CXX']:
> +if CONFIG['CC_TYPE'] == 'clang':

Not GNU_CC/GNU_CXX.

::: toolkit/components/protobuf/moz.build:129
(Diff revision 5)
>          '-Wno-sign-compare',
>          '-Wno-unused-function',
>          '-Wno-unused-local-typedef',
>      ]
>      if CONFIG['CLANG_CXX']:
> +    if CONFIG['CC_TYPE'] == 'clang':

Not GNU_CC/GNU_CXX, and you didn't remove the old line.

::: toolkit/crashreporter/breakpad-client/mac/crash_generation/moz.build:19
(Diff revision 5)
>  LOCAL_INCLUDES += [
>      '/toolkit/crashreporter/breakpad-client',
>      '/toolkit/crashreporter/google-breakpad/src',
>  ]
>  
> -if CONFIG['CLANG_CXX']:
> +if CONFIG['CC_TYPE'] == 'clang':

Not GNU_CC/GNU_CXX

::: toolkit/mozapps/update/updater/updater-common.build:119
(Diff revision 5)
>      'comctl32.dll',
>      'userenv.dll',
>      'wsock32.dll',
>  ]
>  
> -if CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'):

Not GNU_CC/GNU_CXX.
Attachment #8902173 - Flags: review?(mh+mozilla)

Comment 17

2 years ago
mozreview-review
Comment on attachment 8904271 [details]
Bug 1394734 - Replace CONFIG['MSVC'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/176044/#review183054

::: config/external/icu/defs.mozbuild:41
(Diff revision 2)
>  DISABLE_STL_WRAPPING = True
>  ALLOW_COMPILER_WARNINGS = True
>  
>  # We allow compiler warnings, but we can at least cut down on spammy
>  # warnings that get triggered for every file.
> -if CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'clang-cl':

There's another one in this file.

::: gfx/angle/src/libANGLE/moz.build:344
(Diff revision 2)
>          '-Wno-shadow',
>          '-Wno-sign-compare',
>          '-Wno-unknown-pragmas',
>          '-Wno-unreachable-code',
>      ]
> -    if CONFIG['CLANG_CXX']:
> +    if CONFIG['CC_TYPE'] == 'clang':

There's another one in this file.

::: gfx/cairo/cairo/src/moz.build:216
(Diff revision 2)
>  if CONFIG['MOZ_TREE_FREETYPE']:
>      DEFINES['HAVE_FT_LIBRARY_SETLCDFILTER'] = True
>      DEFINES['FT_LCD_FILTER_H'] = '%s/modules/freetype2/include/freetype/ftlcdfil.h' % TOPSRCDIR
>  
>  # Suppress warnings in third-party code.
> -if CONFIG['GNU_CC'] or CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] != 'msvc':

There's another one in this file

::: gfx/cairo/libpixman/src/moz.build:154
(Diff revision 2)
>      CFLAGS += [
>          '-Wno-incompatible-pointer-types',
>          '-Wno-tautological-compare',
>          '-Wno-tautological-constant-out-of-range-compare',
>      ]
> -if CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'clang-cl':

and this file.

::: media/libaom/moz.build:73
(Diff revision 2)
>      if CONFIG['OS_TARGET'] == 'Android':
>          # For cpu-features.h
>          LOCAL_INCLUDES += [
>              '%%%s/sources/android/cpufeatures' % CONFIG['ANDROID_NDK'],
>          ]
> -    if CONFIG['CLANG_CXX']:
> +    if CONFIG['CC_TYPE'] == 'clang':

and this file.

::: media/libav/libavcommon.mozbuild:39
(Diff revision 2)
>          '-Wno-pointer-sign',
>          '-Wno-sign-compare',
>          '-Wno-switch',
>          '-Wno-type-limits',
>      ]
> -if CONFIG['CLANG_CXX'] or CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] in ('clang', 'clang-cl'):

and this one.

::: media/libopus/moz.build
(Diff revision 2)
>      ]
>      ASFLAGS += CONFIG['NEON_FLAGS']
>  
>  # Suppress warnings in third-party code.
> -if CONFIG['GNU_CC']:
> +if CONFIG['CC_TYPE'] == 'clang':
> -    if CONFIG['CLANG_CXX']:

and this one.

::: media/libtheora/moz.build:27
(Diff revision 2)
>  DEFINES['THEORA_DISABLE_ENCODE'] = True
>  
>  # Suppress warnings in third-party code.
>  if CONFIG['CC_TYPE'] != 'msvc':
>      CFLAGS += ['-Wno-type-limits']
> -if CONFIG['CLANG_CXX'] or CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] in ('clang', 'clang-cl'):

and this one.

::: media/libvpx/moz.build:94
(Diff revision 2)
>      SOURCES += [
>          '%%%s/sources/android/cpufeatures/cpu-features.c' % CONFIG['ANDROID_NDK'],
>      ]
>  
>  if CONFIG['CLANG_CL'] or not CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] == 'clang-cl' or CONFIG['CC_TYPE'] != 'msvc':

you're adding a line here.

::: media/webrtc/moz.build:129
(Diff revision 2)
>      GYP_DIRS['trunk/third_party/gflags'].input = 'trunk/third_party/gflags/gflags.gyp'
>      GYP_DIRS['trunk/third_party/gflags'].variables = gyp_vars_copy
>      GYP_DIRS['trunk/third_party/gflags'].sandbox_vars['ALLOW_COMPILER_WARNINGS'] = True
>  
>      if CONFIG['_MSC_VER']:
> +    if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'):

adding a line here too

::: media/webrtc/signaling/gtest/moz.build:55
(Diff revision 2)
>  
>  if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
>      CXXFLAGS += ['-Wno-error=shadow']
>  
>  if CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'):

and here
Attachment #8904271 - Flags: review?(mh+mozilla)

Comment 18

2 years ago
mozreview-review
Comment on attachment 8904272 [details]
Bug 1394734 - Replace CONFIG['CLANG*'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/176046/#review183058

A purely mechanical change would have been way easier to review.
That is:
- mechanically change CONFIG['GNU_CC']/CONFIG['GNU_CXX'] to CONFIG['CC_TYPE'] test (with sed)
- then CONFIG['CLANG_CXX']
- etc.
- and *then* make the adjustments removing redundant stuff.

Because then it's *way* easier to check the last part.

::: gfx/cairo/cairo/src/moz.build:265
(Diff revision 2)
>          '-wd4828', # illegal in the current source character set
>          '-wd4838', # requires a narrowing conversion
>      ]
>  
>  # See bug 386897.
> -if CONFIG['GNU_CC'] and CONFIG['OS_TARGET'] == 'Android' and CONFIG['MOZ_OPTIMIZE']:
> +if CONFIG['CC_TYPE'] in ('clang', 'gcc') and CONFIG['OS_TARGET'] == 'Android' and CONFIG['MOZ_OPTIMIZE']:

should be in the first patch

::: gfx/cairo/libpixman/src/moz.build:158
(Diff revision 2)
>      ]
>  if CONFIG['CC_TYPE'] == 'clang-cl':
>      CFLAGS += [
>          '-Wno-unused-variable',
>      ]
> -if CONFIG['_MSC_VER'] and not CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'msvc' and CONFIG['CC_TYPE'] != 'clang-cl':

second part of the test is useless.

::: media/libaom/moz.build:100
(Diff revision 2)
>      if not CONFIG['MOZ_WEBRTC']:
>          SOURCES += [
>              '%%%s/sources/android/cpufeatures/cpu-features.c' % CONFIG['ANDROID_NDK'],
>          ]
>  
> -if CONFIG['CLANG_CL'] or not CONFIG['_MSC_VER']:
> +if CONFIG['CC_TYPE'] == 'clang-cl' or CONFIG['CC_TYPE'] != 'msvc':

first half of the test is useless.

::: media/libaom/moz.build:113
(Diff revision 2)
>          elif f.endswith('avx.c'):
>              SOURCES[f].flags += ['-mavx']
>          elif f.endswith('avx2.c'):
>              SOURCES[f].flags += ['-mavx2']
>  
> -if CONFIG['_MSC_VER'] and not CONFIG['CLANG_CL']:
> +if CONFIG['CC_TYPE'] == 'msvc' and CONFIG['CC_TYPE'] != 'clang-cl':

second half of the test is useless
Attachment #8904272 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 23

2 years ago
My ugly script:

LIST=$(find . -iname '*build*' -type f|grep -v "\.hg")
sed -i -e "s|if CONFIG\['GNU_CC'\]|if CONFIG['CC_TYPE'] in ('clang', 'gcc')|g" $LIST
sed -i -e "s|if CONFIG\['GNU_CXX'\]|if CONFIG['CC_TYPE'] in ('clang', 'gcc')|g" $LIST
sed -i -e "s|not CONFIG\['GNU_CC'\]:|CONFIG['CC_TYPE'] not in ('gcc', 'clang'):|g" $LIST
sed -i -e "s|if CONFIG\['CLANG_CXX'\] or CONFIG\['GNU_CXX'\]|if CONFIG['CC_TYPE'] in ('clang', 'gcc')|g" $LIST
sed -i -e "s|and CONFIG\['GNU_CC'\]:|and CONFIG['CC_TYPE'] in ('clang', 'gcc'):|g" $LIST
sed -i -e "s|and CONFIG\['GNU_CC'\] and|and CONFIG['CC_TYPE'] in ('clang', 'gcc') and|g" $LIST
sed -i -e "s|(CONFIG\['GNU_CC'\] or|(CONFIG['CC_TYPE'] in ('clang', 'gcc') or|g" $LIST
sed -i -e "s|or CONFIG\['GNU_CXX'\]:|or CONFIG['CC_TYPE'] in ('clang', 'gcc'):|g" $LIST
hg commit -m "Bug 1394734 - Replace CONFIG['GNU_C*'] by CONFIG['CC_TYPE'] r?glandium" .

sed -i -e "s|if CONFIG\['_MSC_VER'\]|if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl')|g" $LIST
sed -i -e "s|if CONFIG\['_MSC_VER'\] and not CONFIG\['CLANG_CL'\]|if CONFIG['CC_TYPE'] == 'msvc'|g" $LIST
sed -i -e "s|if not CONFIG\['_MSC_VER'\]:|if CONFIG['CC_TYPE'] not in ('msvc', 'clang-cl'):|g" $LIST
hg commit -m "Bug 1394734 - Replace CONFIG['MSVC'] by CONFIG['CC_TYPE'] r?glandium" .

sed -i -e "s|if not CONFIG\['_MSC_VER'\] or CONFIG\['CLANG_CL'\]:|if CONFIG['CC_TYPE'] != 'msvc':|g" $LIST
sed -i -e "s|if CONFIG\['CLANG_CL'\]:|if CONFIG['CC_TYPE'] == 'clang-cl':|g" $LIST
sed -i -e "s| and CONFIG\['CLANG_CL'\]:| and CONFIG['CC_TYPE'] == 'clang-cl':|g" $LIST
sed -i -e "s|if CONFIG\['CLANG_CL'\] or not CONFIG\['_MSC_VER'\]|if CONFIG['CC_TYPE'] != 'msvc'|g" $LIST
sed -i -e "s|if CONFIG\['CLANG_CXX'\]:|if CONFIG['CC_TYPE'] == 'clang':|g" $LIST
sed -i -e "s|if CONFIG\['CLANG_CXX'\] or CONFIG\['CLANG_CL'\]:|if CONFIG['CC_TYPE'] in ('clang', 'clang-cl'):|g" $LIST
sed -i -e "s|if CONFIG\['GNU_CC'\] or CONFIG\['CLANG_CL'\]:|if CONFIG['CC_TYPE'] != 'msvc':|g" $LIST
sed -i -e "s|if CONFIG\['CLANG_CXX'\] |if CONFIG['CC_TYPE'] == 'clang' |g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] in ('clang', 'gcc') or CONFIG\['CLANG_CXX'\]|CONFIG['CC_TYPE'] in ('clang', 'gcc')|g" $LIST
sed -i -e "s|and not CONFIG\['CLANG_CXX'\]|and CONFIG['CC_TYPE'] != 'clang'|g" $LIST
hg commit -m "Bug 1394734 - Replace CONFIG['CLANG*'] by CONFIG['CC_TYPE'] r?glandium" .

sed -i -e "s|and not CONFIG\['CLANG_CL'\]|and CONFIG['CC_TYPE'] != 'clang-cl'|g" $LIST
sed -i -e "s|if CONFIG\['CC_TYPE'\] in ('msvc', 'clang-cl') and CONFIG\['CC_TYPE'\] != 'clang-cl':|if CONFIG['CC_TYPE'] == 'msvc':|g" $LIST
sed -i -e "s|if CONFIG\['CC_TYPE'\] in ('clang', 'gcc') and not CONFIG\['CLANG_CXX'\] and CONFIG\['CC_TYPE'\] != 'clang-cl':|if CONFIG['CC_TYPE'] == 'gcc':|g" $LIST
sed -i -e "s|if CONFIG\['CC_TYPE'\] in ('msvc', 'clang-cl') and not CONFIG\['CLANG_CL'\]:|if CONFIG['CC_TYPE'] == 'msvc':|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] in ('clang', 'gcc') or CONFIG\['CLANG_CL'\]|CONFIG['CC_TYPE'] in ('clang', 'clang-cl', 'gcc')|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] == 'clang' or CONFIG\['CLANG_CL'\] or CONFIG\['CC_TYPE'\] in ('clang', 'gcc')|CONFIG['CC_TYPE'] in ('clang', 'clang_cl', 'gcc')|g" $LIST
sed -i -e "s|if CONFIG\['CC_TYPE'\] in ('clang', 'gcc') and CONFIG\['CC_TYPE'\] != 'clang':|if CONFIG['CC_TYPE'] in ('clang', 'gcc'):|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] in ('clang', 'gcc') and CONFIG\['CC_TYPE'\] != 'clang' and CONFIG\['CC_TYPE'\] != 'clang-cl'|CONFIG['CC_TYPE'] == 'gcc'|g" $LIST
hg commit -m "Bug 1394734 - Fix some various corner cases r?glandium" .
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
I was realizing that with my patches in bug 1397263 the only things left in toolchain.m4's `MOZ_TOOL_VARIABLES` macro were GNU_{CC,CXX}, CLANG_{CC,CXX}, CLANG_CL and GCC_USE_GNU_LD. If your patches here land then we could simply remove setting most of those, and the last one could be a one-line set_config in toolchain.configure.
(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> sed -i -e "s|not CONFIG\['GNU_CC'\]:|CONFIG['CC_TYPE'] not in ('gcc',
> 'clang'):|g" $LIST

This one is needlessly in a different order.

That said, what I had more in mind was something like these to start with:

sed -i -e "s|not CONFIG\['GNU_CC'\]|CONFIG['CC_TYPE'] not in ('clang', 'gcc')|g" $LIST
sed -i -e "s|not CONFIG\['GNU_CXX'\]|CONFIG['CC_TYPE'] not in ('clang', 'gcc')|g" $LIST
sed -i -e "s|CONFIG\['GNU_CXX'\]|CONFIG['CC_TYPE'] in ('clang', 'gcc')|g" $LIST
sed -i -e "s|CONFIG\['GNU_CC'\]|CONFIG['CC_TYPE'] in ('clang', 'gcc')|g" $LIST
commit

sed -i -e "s|not CONFIG\['_MSC_VER'\]|CONFIG['CC_TYPE'] not in ('msvc', 'clang-cl')|g" $LIST
sed -i -e "s|CONFIG\['_MSC_VER'\]|CONFIG['CC_TYPE'] in ('msvc', 'clang-cl')|g" $LIST
commit

sed -i -e "s|not CONFIG\['CLANG_CL'\]|CONFIG['CC_TYPE'] != 'clang-cl'|g" $LIST
sed -i -e "s|CONFIG\['CLANG_CL'\]|CONFIG['CC_TYPE'] == 'clang-cl'|g" $LIST
sed -i -e "s|not CONFIG\['CLANG_CXX'\]|CONFIG['CC_TYPE'] != 'clang'|g" $LIST
sed -i -e "s|CONFIG\['CLANG_CXX'\]|CONFIG['CC_TYPE'] == 'clang'|g" $LIST
commit

Then there aren't too many left, that can be edited by hand and verified more easily, but can also be fixed up with sed too:

sed -i -e "s|CONFIG\['CC_TYPE'\] in (\('clang'\), \('gcc'\)) or CONFIG\['CC_TYPE'\] == \('clang-cl'\)|CONFIG['CC_TYPE'] in (\1, \3, \2)|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] == \('clang-cl'\) or CONFIG\['CC_TYPE'\] in (\('clang'\), \('gcc'\))|CONFIG['CC_TYPE'] in (\2, \1, \3)|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] == \('[^']*'\) or CONFIG\['CC_TYPE'\] == \('[^']*'\)|CONFIG['CC_TYPE'] in (\1, \2)|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] == \('[^']*'\) or \(CONFIG\['CC_TYPE'\] in ([^)]*\1[^)]*)\)|\2|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] in (\('[^']*'\), \('[^']*'\)) and CONFIG\['CC_TYPE'\] != \2|CONFIG['CC_TYPE'] == \1|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] in (\('[^']*'\), \('[^']*'\)) and CONFIG\['CC_TYPE'\] != \1|CONFIG['CC_TYPE'] == \2|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] in (\('[^']*'\), \('[^']*'\)) or CONFIG\['CC_TYPE'\] == \1|CONFIG['CC_TYPE'] in (\1, \2)|g" $LIST
sed -i -e "s|\(CONFIG\['CC_TYPE'\] == 'gcc'\) and CONFIG\['CC_TYPE'\] != 'clang-cl'|\1|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] == 'clang-cl' or CONFIG\['CC_TYPE'\] not in ('msvc', 'clang-cl')|CONFIG['CC_TYPE'] != 'msvc'|g" $LIST
sed -i -e "s|CONFIG\['CC_TYPE'\] not in ('msvc', 'clang-cl') or CONFIG\['CC_TYPE'\] == 'clang-cl'|CONFIG['CC_TYPE'] != 'msvc'|g" $LIST

Which yields the following difference against your patch queue:

diff --git a/security/pkix/test/gtest/moz.build b/security/pkix/test/gtest/moz.build
index f04678413a21f..b60c77354595f 100644
--- a/security/pkix/test/gtest/moz.build
+++ b/security/pkix/test/gtest/moz.build
@@ -34,17 +34,17 @@ LOCAL_INCLUDES += [
 ]
 
 FINAL_LIBRARY = 'xul-gtest'
 
 include('../../warnings.mozbuild')
 
 # GTest uses a variadic macro in a questionable way and it doesn't seem to be
 # possible to selectively disable just that error when -pedantic-errors is set.
-if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
+if CONFIG['CC_TYPE'] == 'gcc':
   CXXFLAGS.remove('-pedantic-errors')
 
 # These warnings are disabled in order to minimize the amount of boilerplate
 # required to implement tests, and/or because they originate in the GTest
 # framework in a way we cannot otherwise work around.
 if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
   CXXFLAGS += [
     '-Wno-error=shadow',
diff --git a/toolkit/components/jsoncpp/src/lib_json/moz.build b/toolkit/components/jsoncpp/src/lib_json/moz.build
index cc4882c578593..6920229e1ea1f 100644
--- a/toolkit/components/jsoncpp/src/lib_json/moz.build
+++ b/toolkit/components/jsoncpp/src/lib_json/moz.build
@@ -42,12 +42,12 @@ elif CONFIG['CC_TYPE'] in ('clang', 'gcc'):
         '-Wno-shadow',
     ]
 
 if CONFIG['CC_TYPE'] in ('clang', 'clang-cl'):
     CXXFLAGS += [
         '-Wno-c++11-narrowing',
     ]
 
-if CONFIG['CC_TYPE'] in ('clang', 'clang_cl', 'gcc'):
+if CONFIG['CC_TYPE'] in ('clang', 'clang-cl', 'gcc'):
     CXXFLAGS += [
         '-Wno-implicit-fallthrough',
     ]

The latter is your version having clang_cl vs. mine having clang-cl, and the former was `if CONFIG['GNU_CXX'] and not CONFIG['CLANG_CXX']:` originally, so my version is right.
Attachment #8902173 - Flags: review?(mh+mozilla)
Attachment #8904271 - Flags: review?(mh+mozilla)
Attachment #8904272 - Flags: review?(mh+mozilla)
Attachment #8934970 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 34

2 years ago
mozreview-review
Comment on attachment 8902173 [details]
Bug 1394734 - Replace CONFIG['GNU_C*'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/173644/#review212084
Attachment #8902173 - Flags: review?(mh+mozilla) → review+

Comment 35

2 years ago
mozreview-review
Comment on attachment 8904271 [details]
Bug 1394734 - Replace CONFIG['MSVC'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/176044/#review212088
Attachment #8904271 - Flags: review?(mh+mozilla) → review+

Comment 36

2 years ago
mozreview-review
Comment on attachment 8904272 [details]
Bug 1394734 - Replace CONFIG['CLANG*'] by CONFIG['CC_TYPE']

https://reviewboard.mozilla.org/r/176046/#review212090
Attachment #8904272 - Flags: review?(mh+mozilla) → review+

Comment 37

2 years ago
mozreview-review
Comment on attachment 8934970 [details]
Bug 1394734 - Simplify various corner cases

https://reviewboard.mozilla.org/r/205912/#review212092

::: commit-message-0e677:1
(Diff revision 3)
> +Bug 1394734 - Fix some various corner cases r?glandium

s/Fix some/Simplify/
Attachment #8934970 - Flags: review?(mh+mozilla) → review+
Comment hidden (mozreview-request)

Comment 39

2 years ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 5988d7d7bbd7 -d 7642e86127fc: rebasing 438204:5988d7d7bbd7 "Bug 1394734 - Replace CONFIG['GNU_C*'] by CONFIG['CC_TYPE'] r=glandium"
merging build/gecko_templates.mozbuild
merging dom/media/webspeech/synth/moz.build
merging gfx/2d/moz.build
rebasing 438205:9183b616c9fc "Bug 1394734 - Replace CONFIG['MSVC'] by CONFIG['CC_TYPE'] r=glandium"
merging memory/fallible/moz.build
merging widget/windows/moz.build
merging xpcom/glue/standalone/moz.build
warning: conflicts while merging memory/fallible/moz.build! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 44

2 years ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5ec58e0e3a34
Replace CONFIG['GNU_C*'] by CONFIG['CC_TYPE'] r=glandium
https://hg.mozilla.org/integration/autoland/rev/2d15016c5ede
Replace CONFIG['MSVC'] by CONFIG['CC_TYPE'] r=glandium
https://hg.mozilla.org/integration/autoland/rev/e9a659f1e7a1
Replace CONFIG['CLANG*'] by CONFIG['CC_TYPE'] r=glandium
https://hg.mozilla.org/integration/autoland/rev/8ee8daca740b
Simplify various corner cases r=glandium

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.