[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

2023-09-05 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D159115#4638325 , @v.g.vassilev 
wrote:

> In D159115#4638323 , @mgorny wrote:
>
>> Changing the type from `unsigned long long` to `uintptr_t` fix the test for 
>> me.
>
> Ah! Nice catch! Can you commit the fix?

Done, thanks to @mgorny for the testing and thank you @v.g.vassilev for the 
quick ack!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159115/new/

https://reviews.llvm.org/D159115

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159173: [Driver] Report warnings for unclaimed TargetSpecific options for assembler input

2023-08-31 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159173/new/

https://reviews.llvm.org/D159173

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D143241: [Clang] Reset FP options before function instantiations

2023-07-11 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

During our regular snapshot testing, we've hit this in Gentoo on 32-bit Linux 
x86: https://github.com/llvm/llvm-project/issues/63704#issuecomment-1631791518.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143241/new/

https://reviews.llvm.org/D143241

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150930: [Driver] Accept and ignore -fno-lifetime-dse argument

2023-05-18 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

This is just a draft which I've only briefly tested so far. I'm wondering if 
this is the right place to put the option, as i've not had to write anything in 
tablegen before.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150930/new/

https://reviews.llvm.org/D150930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150930: [Driver] Accept and ignore -fno-lifetime-dse argument

2023-05-18 Thread Sam James via Phabricator via cfe-commits
thesamesam created this revision.
thesamesam added reviewers: MaskRay, jhuber6, tstellar.
Herald added a project: All.
thesamesam requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Since 47f5c54f997a59bb2c65abe6b8b811f6e7553456 
, we pass 
-fno-lifetime-dse
when building LLVM with GCC.

Unfortunately, this impacts projects which build LLVM using GCC who then try
to use e.g. clang-tidy because of the flag leaking into compile_commands.json.

While we're trying to stop adding these, given we're passing the flag ourselves,
I don't think we have much of a choice here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150930

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4940,6 +4940,7 @@
 defm default_inline : BooleanFFlag<"default-inline">, 
Group;
 defm fat_lto_objects : BooleanFFlag<"fat-lto-objects">, 
Group;
 defm float_store : BooleanFFlag<"float-store">, 
Group;
+defm fno_lifetime_dse : BooleanFFlag<"lifetime-dse">, IgnoredGCCCompat;
 defm friend_injection : BooleanFFlag<"friend-injection">, 
Group;
 defm function_attribute_list : BooleanFFlag<"function-attribute-list">, 
Group;
 defm gcse : BooleanFFlag<"gcse">, 
Group;


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4940,6 +4940,7 @@
 defm default_inline : BooleanFFlag<"default-inline">, Group;
 defm fat_lto_objects : BooleanFFlag<"fat-lto-objects">, Group;
 defm float_store : BooleanFFlag<"float-store">, Group;
+defm fno_lifetime_dse : BooleanFFlag<"lifetime-dse">, IgnoredGCCCompat;
 defm friend_injection : BooleanFFlag<"friend-injection">, Group;
 defm function_attribute_list : BooleanFFlag<"function-attribute-list">, Group;
 defm gcse : BooleanFFlag<"gcse">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150582: [clangd] Fix test failure when it's built with compiler flags unknown by clang

2023-05-16 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

Sure & thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150582/new/

https://reviews.llvm.org/D150582

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150582: [clangd] Fix test failure when it's built with compiler flags unknown by clang

2023-05-15 Thread Sam James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4ddae8b94139: [clangd] Fix test failure when its built 
with compiler flags unknown by clang (authored by xry111, committed by 
thesamesam).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150582/new/

https://reviews.llvm.org/D150582

Files:
  clang-tools-extra/clangd/test/CMakeLists.txt
  clang-tools-extra/clangd/test/compile_commands.json


Index: clang-tools-extra/clangd/test/compile_commands.json
===
--- /dev/null
+++ clang-tools-extra/clangd/test/compile_commands.json
@@ -0,0 +1 @@
+[]
Index: clang-tools-extra/clangd/test/CMakeLists.txt
===
--- clang-tools-extra/clangd/test/CMakeLists.txt
+++ clang-tools-extra/clangd/test/CMakeLists.txt
@@ -28,6 +28,16 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
   )
 
+# Copy an empty compile_commands.json to override the compile_commands.json
+# in the top level build directory.  Or if a clangd test involves creating a
+# temporary source file in the build directory and run clangd to check it,
+# it can pick up unrecognizable command options when LLVM is built with
+# another compiler or a different version of Clang.
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/compile_commands.json
+  ${CMAKE_CURRENT_BINARY_DIR}/compile_commands.json
+)
+
 add_lit_testsuite(check-clangd "Running the Clangd regression tests"
   # clangd doesn't put unittest configs in test/unit like every other project.
   # Because of that, this needs to pass two folders here, while every other


Index: clang-tools-extra/clangd/test/compile_commands.json
===
--- /dev/null
+++ clang-tools-extra/clangd/test/compile_commands.json
@@ -0,0 +1 @@
+[]
Index: clang-tools-extra/clangd/test/CMakeLists.txt
===
--- clang-tools-extra/clangd/test/CMakeLists.txt
+++ clang-tools-extra/clangd/test/CMakeLists.txt
@@ -28,6 +28,16 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.cfg.py
   )
 
+# Copy an empty compile_commands.json to override the compile_commands.json
+# in the top level build directory.  Or if a clangd test involves creating a
+# temporary source file in the build directory and run clangd to check it,
+# it can pick up unrecognizable command options when LLVM is built with
+# another compiler or a different version of Clang.
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/compile_commands.json
+  ${CMAKE_CURRENT_BINARY_DIR}/compile_commands.json
+)
+
 add_lit_testsuite(check-clangd "Running the Clangd regression tests"
   # clangd doesn't put unittest configs in test/unit like every other project.
   # Because of that, this needs to pass two folders here, while every other
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150582: [clangd] Fix test failure when it's built with compiler flags unknown by clang

2023-05-15 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

(I even struggled to find docs for the fix we used for clang-tidy, although it 
obviously works.)

Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150582/new/

https://reviews.llvm.org/D150582

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

2023-05-11 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

Adding to the concerns raised above, I don't think we're there yet. See 
https://bugs.gentoo.org/buglist.cgi?quicksearch=enum-constexpr-conversion_id=6843355
 and keep in mind that a bunch of stuff isn't buildable with Clang 16 yet 
anyway so I expect a bunch more to be broken on top of that.

Also, e.g. gdb isn't fixed in a release yet.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150226/new/

https://reviews.llvm.org/D150226

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D148496: [compiler-rt] [test] Mark dfsan tests XFAIL on glibc-2.37

2023-04-17 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

Yes, please. Obviously we want to fix it properly but right now it's a known 
issue and it's noise when checking for regressions weekly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148496/new/

https://reviews.llvm.org/D148496

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2023-03-22 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D133375#4214214 , @tambre wrote:

> I've now run into a snag with this. Having `clang++.cfg` with `-std=c++2b` 
> makes `/usr/bin/clang++ -E -dM -` fail with `error: invalid argument 
> '-std=c++2b' not allowed with 'C'`.
> This is invoked by Meson during its configuration process and causes it to 
> fail.
>
> Am I doing something wrong?

Filed https://github.com/llvm/llvm-project/issues/61641, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133375/new/

https://reviews.llvm.org/D133375

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D144540: [Clang] [Doc] Explicitly note noreturn bug as a miscompilation

2023-02-22 Thread Sam James via Phabricator via cfe-commits
thesamesam closed this revision.
thesamesam added a comment.

Thanks! Merged.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144540/new/

https://reviews.llvm.org/D144540

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D144540: [Clang] [Doc] Explicitly note noreturn bug as a miscompilation

2023-02-22 Thread Sam James via Phabricator via cfe-commits
thesamesam created this revision.
thesamesam added a reviewer: aaron.ballman.
Herald added a project: All.
thesamesam requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is for 16.x.

Bug: https://github.com/llvm/llvm-project/issues/59792


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144540

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -345,7 +345,7 @@
   character sequence that differed from the primary source file.
   `Issue 59736 `_
 - In C mode, when ``e1`` has ``__attribute__((noreturn))`` but ``e2`` doesn't,
-  ``(c ? e1 : e2)`` is no longer considered noreturn.
+  ``(c ? e1 : e2)`` is no longer considered noreturn, fixing a miscompilation.
   `Issue 59792 `_
 - Fix an issue that makes Clang crash on lambda template parameters. This fixes
   `Issue 57960 `_


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -345,7 +345,7 @@
   character sequence that differed from the primary source file.
   `Issue 59736 `_
 - In C mode, when ``e1`` has ``__attribute__((noreturn))`` but ``e2`` doesn't,
-  ``(c ? e1 : e2)`` is no longer considered noreturn.
+  ``(c ? e1 : e2)`` is no longer considered noreturn, fixing a miscompilation.
   `Issue 59792 `_
 - Fix an issue that makes Clang crash on lambda template parameters. This fixes
   `Issue 57960 `_
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-01-30 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

I both like it and can't foresee any issues from the vendor side.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142826/new/

https://reviews.llvm.org/D142826

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

2023-01-23 Thread Sam James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG136f77805fd8: [Clang] [Python] Fix tests when default config 
file contains -include (authored by thesamesam).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141248/new/

https://reviews.llvm.org/D141248

Files:
  clang/bindings/python/tests/CMakeLists.txt


Index: clang/bindings/python/tests/CMakeLists.txt
===
--- clang/bindings/python/tests/CMakeLists.txt
+++ clang/bindings/python/tests/CMakeLists.txt
@@ -1,7 +1,10 @@
 # Test target to run Python test suite from main build.
 
+# Avoid configurations including '-include' from interfering with
+# our tests by setting CLANG_NO_DEFAULT_CONFIG.
 add_custom_target(check-clang-python
 COMMAND ${CMAKE_COMMAND} -E env
+CLANG_NO_DEFAULT_CONFIG=1
 CLANG_LIBRARY_PATH=$
 "${Python3_EXECUTABLE}" -m unittest discover
 DEPENDS libclang


Index: clang/bindings/python/tests/CMakeLists.txt
===
--- clang/bindings/python/tests/CMakeLists.txt
+++ clang/bindings/python/tests/CMakeLists.txt
@@ -1,7 +1,10 @@
 # Test target to run Python test suite from main build.
 
+# Avoid configurations including '-include' from interfering with
+# our tests by setting CLANG_NO_DEFAULT_CONFIG.
 add_custom_target(check-clang-python
 COMMAND ${CMAKE_COMMAND} -E env
+CLANG_NO_DEFAULT_CONFIG=1
 CLANG_LIBRARY_PATH=$
 "${Python3_EXECUTABLE}" -m unittest discover
 DEPENDS libclang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

2023-01-22 Thread Sam James via Phabricator via cfe-commits
thesamesam updated this revision to Diff 491220.
thesamesam added a comment.

Switch to setting CLANG_NO_DEFAULT_CONFIG via CMake instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141248/new/

https://reviews.llvm.org/D141248

Files:
  clang/bindings/python/tests/CMakeLists.txt


Index: clang/bindings/python/tests/CMakeLists.txt
===
--- clang/bindings/python/tests/CMakeLists.txt
+++ clang/bindings/python/tests/CMakeLists.txt
@@ -1,7 +1,10 @@
 # Test target to run Python test suite from main build.
 
+# Avoid configurations including '-include' from interfering with
+# our tests by setting CLANG_NO_DEFAULT_CONFIG.
 add_custom_target(check-clang-python
 COMMAND ${CMAKE_COMMAND} -E env
+CLANG_NO_DEFAULT_CONFIG=1
 CLANG_LIBRARY_PATH=$
 "${Python3_EXECUTABLE}" -m unittest discover
 DEPENDS libclang


Index: clang/bindings/python/tests/CMakeLists.txt
===
--- clang/bindings/python/tests/CMakeLists.txt
+++ clang/bindings/python/tests/CMakeLists.txt
@@ -1,7 +1,10 @@
 # Test target to run Python test suite from main build.
 
+# Avoid configurations including '-include' from interfering with
+# our tests by setting CLANG_NO_DEFAULT_CONFIG.
 add_custom_target(check-clang-python
 COMMAND ${CMAKE_COMMAND} -E env
+CLANG_NO_DEFAULT_CONFIG=1
 CLANG_LIBRARY_PATH=$
 "${Python3_EXECUTABLE}" -m unittest discover
 DEPENDS libclang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

2023-01-22 Thread Sam James via Phabricator via cfe-commits
thesamesam added inline comments.



Comment at: clang/bindings/python/tests/cindex/util.py:82
+# our tests.
+os.environ["CLANG_NO_DEFAULT_CONFIG"] = "1"
 

xen0n wrote:
> This is essentially an import-time side effect, IMO it could be better to 
> instead put the envvar provision in 
> `clang/bindings/python/tests/CMakeLists.txt`: the `add_custom_target` there 
> already is making use of `env` so should be relatively easy to stuff this 
> there too.
I hesitated to do that because it doesn't vary with the environment at all (the 
test should never really be run with custom configs), but given you've 
suggested it, that tips the balance on me being undecided - so I'll implement 
that now. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141248/new/

https://reviews.llvm.org/D141248

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

2023-01-21 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

This is currently holding back further testing on our end which is concerning 
me a bit, especially as we approach the branch point. Could you revert this 
please if a fix isn't imminent? Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

2023-01-08 Thread Sam James via Phabricator via cfe-commits
thesamesam updated this revision to Diff 487296.
thesamesam added a comment.

Add comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141248/new/

https://reviews.llvm.org/D141248

Files:
  clang/bindings/python/tests/cindex/util.py


Index: clang/bindings/python/tests/cindex/util.py
===
--- clang/bindings/python/tests/cindex/util.py
+++ clang/bindings/python/tests/cindex/util.py
@@ -77,6 +77,9 @@
 
 return cursors
 
+# Avoid configurations including '-include' from interfering with
+# our tests.
+os.environ["CLANG_NO_DEFAULT_CONFIG"] = "1"
 
 skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
 "Requires file system path protocol / 
Python 3.6+")


Index: clang/bindings/python/tests/cindex/util.py
===
--- clang/bindings/python/tests/cindex/util.py
+++ clang/bindings/python/tests/cindex/util.py
@@ -77,6 +77,9 @@
 
 return cursors
 
+# Avoid configurations including '-include' from interfering with
+# our tests.
+os.environ["CLANG_NO_DEFAULT_CONFIG"] = "1"
 
 skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
 "Requires file system path protocol / Python 3.6+")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

2023-01-08 Thread Sam James via Phabricator via cfe-commits
thesamesam updated this revision to Diff 487295.
thesamesam added a comment.

Set CLANG_NO_DEFAULT_CONFIG in environment instead to reduce
cat-and-mouse issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141248/new/

https://reviews.llvm.org/D141248

Files:
  clang/bindings/python/tests/cindex/util.py


Index: clang/bindings/python/tests/cindex/util.py
===
--- clang/bindings/python/tests/cindex/util.py
+++ clang/bindings/python/tests/cindex/util.py
@@ -77,6 +77,7 @@
 
 return cursors
 
+os.environ["CLANG_NO_DEFAULT_CONFIG"] = "1"
 
 skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
 "Requires file system path protocol / 
Python 3.6+")


Index: clang/bindings/python/tests/cindex/util.py
===
--- clang/bindings/python/tests/cindex/util.py
+++ clang/bindings/python/tests/cindex/util.py
@@ -77,6 +77,7 @@
 
 return cursors
 
+os.environ["CLANG_NO_DEFAULT_CONFIG"] = "1"
 
 skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
 "Requires file system path protocol / Python 3.6+")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D141248: [Clang] [Python] Fix tests when default config file contains -include

2023-01-08 Thread Sam James via Phabricator via cfe-commits
thesamesam created this revision.
thesamesam added a reviewer: mgorny.
Herald added a subscriber: arphaman.
Herald added a project: All.
thesamesam requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In Gentoo, we make use of Clang's recently-enhanced config file support
and add a default include to `clang` invocations using '-include ...'.

This breaks clang-python tests like so:

  ==
  ERROR: test_includes (tests.cindex.test_translation_unit.TestTranslationUnit)
  --
  Traceback (most recent call last):
File 
"/var/tmp/portage/dev-python/clang-python-15.0.6/work/clang/bindings/python/tests/cindex/test_translation_unit.py",
 line 145, in test_includes
  eq(i[0], i[1])
File 
"/var/tmp/portage/dev-python/clang-python-15.0.6/work/clang/bindings/python/tests/cindex/test_translation_unit.py",
 line 132, in eq
  self.assert_normpaths_equal(expected[0], actual.source.name)
  AttributeError: 'NoneType' object has no attribute 'name'
  
  ==
  FAIL: test_inclusion_directive 
(tests.cindex.test_translation_unit.TestTranslationUnit)
  --
  Traceback (most recent call last):
File 
"/var/tmp/portage/dev-python/clang-python-15.0.6/work/clang/bindings/python/tests/cindex/test_translation_unit.py",
 line 157, in test_inclusion_directive
  self.assert_normpaths_equal(i[0], i[1])
File 
"/var/tmp/portage/dev-python/clang-python-15.0.6/work/clang/bindings/python/tests/cindex/test_translation_unit.py",
 line 126, in assert_normpaths_equal
  self.assertEqual(os.path.normpath(path1),
  AssertionError: '/var/tmp/portage/dev-python/clang-python-1[58 chars]r1.h' != 
'/usr/include/gentoo/fortify.h'
  - 
/var/tmp/portage/dev-python/clang-python-15.0.6/work/clang/bindings/python/tests/cindex/INPUTS/header1.h
  + /usr/include/gentoo/fortify.h

Disable using the default Clang configuration files on the system, like
we did for other tests.

Bug: https://bugs.gentoo.org/890204
Signed-off-by: Sam James 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141248

Files:
  clang/bindings/python/tests/cindex/test_translation_unit.py


Index: clang/bindings/python/tests/cindex/test_translation_unit.py
===
--- clang/bindings/python/tests/cindex/test_translation_unit.py
+++ clang/bindings/python/tests/cindex/test_translation_unit.py
@@ -140,7 +140,7 @@
 h3 = os.path.join(kInputsDir, "header3.h")
 inc = [(src, h1), (h1, h3), (src, h2), (h2, h3)]
 
-tu = TranslationUnit.from_source(src)
+tu = TranslationUnit.from_source(src, ["--no-default-config"])
 for i in zip(inc, tu.get_includes()):
 eq(i[0], i[1])
 
@@ -151,7 +151,7 @@
 h3 = os.path.join(kInputsDir, "header3.h")
 inc = [h1, h3, h2, h3, h1]
 
-tu = TranslationUnit.from_source(src, 
options=TranslationUnit.PARSE_DETAILED_PROCESSING_RECORD)
+tu = TranslationUnit.from_source(src, ["--no-default-config"], 
options=TranslationUnit.PARSE_DETAILED_PROCESSING_RECORD)
 inclusion_directive_files = [c.get_included_file().name for c in 
tu.cursor.get_children() if c.kind == CursorKind.INCLUSION_DIRECTIVE]
 for i in zip(inc, inclusion_directive_files):
 self.assert_normpaths_equal(i[0], i[1])


Index: clang/bindings/python/tests/cindex/test_translation_unit.py
===
--- clang/bindings/python/tests/cindex/test_translation_unit.py
+++ clang/bindings/python/tests/cindex/test_translation_unit.py
@@ -140,7 +140,7 @@
 h3 = os.path.join(kInputsDir, "header3.h")
 inc = [(src, h1), (h1, h3), (src, h2), (h2, h3)]
 
-tu = TranslationUnit.from_source(src)
+tu = TranslationUnit.from_source(src, ["--no-default-config"])
 for i in zip(inc, tu.get_includes()):
 eq(i[0], i[1])
 
@@ -151,7 +151,7 @@
 h3 = os.path.join(kInputsDir, "header3.h")
 inc = [h1, h3, h2, h3, h1]
 
-tu = TranslationUnit.from_source(src, options=TranslationUnit.PARSE_DETAILED_PROCESSING_RECORD)
+tu = TranslationUnit.from_source(src, ["--no-default-config"], options=TranslationUnit.PARSE_DETAILED_PROCESSING_RECORD)
 inclusion_directive_files = [c.get_included_file().name for c in tu.cursor.get_children() if c.kind == CursorKind.INCLUSION_DIRECTIVE]
 for i in zip(inc, inclusion_directive_files):
 self.assert_normpaths_equal(i[0], i[1])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

2023-01-04 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

Confirmed this patch fixes the miscompile.

Right. While the spec side needs discussion (and almost certainly addressing!), 
I don't think anyone in the real world is expecting the Clang behaviour vs the 
GCC one. The Clang one as-is is the one that's less conservative, as 
aggressively imposing Noreturn can only lead to something breaking.

I don't have ICC available to test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140868/new/

https://reviews.llvm.org/D140868

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136283: [clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

2022-12-07 Thread Sam James via Phabricator via cfe-commits
thesamesam abandoned this revision.
thesamesam added a comment.

Done, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136283/new/

https://reviews.llvm.org/D136283

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138520: [clangd] Make decision forest model optional

2022-11-22 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

Works on PPC.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138520/new/

https://reviews.llvm.org/D138520

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136283: [clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

2022-11-15 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D136283#3911832 , @sammccall wrote:

> I don't think we should add significant build-system complexity here in order 
> to support the completion model on ppc32.
> Do people actually use clangd on ppc32 machines? (The linked bug calls this a 
> clang build failure, but this code is not part of clang).
>
> If we need to support building on this toolchain, then we should probably 
> just disable the decision forest model entirely and use the heuristic 
> completion scorer instead.

I've had a think about this and I'm not aware of anyone using clangd on ppc32. 
So I agree this patch isn't worth the complexity. But it'd be nice to just 
switch to the heuristic scorer if it's somewhat easy rather than leave it 
broken (or bother having to wire up some "PPC is not supported" message). I've 
got no idea how to do that though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136283/new/

https://reviews.llvm.org/D136283

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137511: [PPC] Undefine __ppc64__ to match GCC

2022-11-06 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

This needs to go in Breaking Changes in the release notes, not least so we can 
link to it when updating upstreams.

What do you mean by "while the darwin support has been removed from 
llvm-project."? I don't think that's the case, if you mean that LLVM.org's LLVM 
lacks support for Darwin.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137511/new/

https://reviews.llvm.org/D137511

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136283: [clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

2022-11-02 Thread Sam James via Phabricator via cfe-commits
thesamesam updated this revision to Diff 472807.
thesamesam added a comment.

Use PARENT_SCOPE.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136283/new/

https://reviews.llvm.org/D136283

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/quality/CompletionModel.cmake
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py
  clang-tools-extra/clangd/unittests/CMakeLists.txt

Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -20,7 +20,11 @@
 endif()
 
 include(${CMAKE_CURRENT_SOURCE_DIR}/../quality/CompletionModel.cmake)
-gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model DecisionForestRuntimeTest ::ns1::ns2::test::Example)
+gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model
+  DecisionForestRuntimeTest
+  ::ns1::ns2::test::Example
+  DecisionForestRuntimeTest_header_file
+  DecisionForestRuntimeTest_cpp_files)
 
 add_custom_target(ClangdUnitTests)
 add_unittest(ClangdUnitTests ClangdTests
@@ -95,7 +99,7 @@
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
-  ${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest.cpp
+  ${DecisionForestRuntimeTest_cpp_files}
 
   support/CancellationTests.cpp
   support/ContextTests.cpp
@@ -134,9 +138,9 @@
   $
   )
 
-# Include generated ComletionModel headers.
+# Include generated Completion Model header.
 target_include_directories(ClangdTests PUBLIC
-  $
+  $
 )
 
 clang_target_link_libraries(ClangdTests
Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -1,13 +1,15 @@
 """Code generator for Code Completion Model Inference.
 
 Tool runs on the Decision Forest model defined in {model} directory.
-It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp
-The generated files defines the Example class named {cpp_class} having all the features as class members.
+It generates files: {output_dir}/{filename}.h, {output_dir}/{filename}.cpp,
+and {output_dir}/{filename}{number}.cpp for each Decision Tree.
+The generated files define the Example class named {cpp_class} having all the features as class members.
 The generated runtime provides an `Evaluate` function which can be used to score a code completion candidate.
 """
 
 import argparse
 import json
+import os
 import struct
 
 
@@ -194,34 +196,36 @@
 `float Evaluate(const {Example}&)` function. This function can be 
 used to score an Example."""
 
-code = ""
+functions_codes = {}
 
 # Generate evaluation function of each tree.
-code += "namespace {\n"
 tree_num = 0
 for tree_json in forest_json:
-code += "LLVM_ATTRIBUTE_NOINLINE float EvaluateTree%d(const %s& E) {\n" % (tree_num, cpp_class.name)
-code += "  " + \
+functions_codes[f"{tree_num}"] = "LLVM_ATTRIBUTE_NOINLINE float EvaluateTree%d(const %s& E) {\n" % (tree_num, cpp_class.name)
+functions_codes[f"{tree_num}"] += "  " + \
 "\n  ".join(
 tree(tree_json, tree_num=tree_num, node_num=0)[0]) + "\n"
-code += "}\n\n"
+functions_codes[f"{tree_num}"] += "}\n"
 tree_num += 1
-code += "} // namespace\n\n"
 
 # Combine the scores of all trees in the final function.
 # MSAN will timeout if these functions are inlined.
-code += "float Evaluate(const %s& E) {\n" % cpp_class.name
-code += "  float Score = 0;\n"
+final_function_code = ""
 for tree_num in range(len(forest_json)):
-code += "  Score += EvaluateTree%d(E);\n" % tree_num
-code += "  return Score;\n"
-code += "}\n"
+final_function_code += "float EvaluateTree%d(const %s& E);\n" % (tree_num, cpp_class.name)
+final_function_code += "\n"
+final_function_code += "float Evaluate(const %s& E) {\n" % cpp_class.name
+final_function_code += "  float Score = 0;\n"
+for tree_num in range(len(forest_json)):
+final_function_code += "  Score += EvaluateTree%d(E);\n" % tree_num
+final_function_code += "  return Score;\n"
+final_function_code += "}\n"
 
-return code
+return functions_codes, final_function_code
 
 
 def gen_cpp_code(forest_json, features_json, filename, cpp_class):
-"""Generates code for the .cpp file."""
+"""Generates code for the .cpp files."""
 # Headers
 # Required by OrderEncode(float F).
 angled_include = [
@@ -242,11 +246,11 @@
 for feature in features_json
 if feature["kind"] == "ENUM")
 nl = "\n"
-return """%s
 
-%s
+functions_codes, final_function_code = evaluate_func(forest_json, cpp_class)
 
-#define BIT(X) (1LL << X)
+ 

[PATCH] D137043: [clang] add implicit include for Linux/gnu compatibility

2022-10-30 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

Thanks for the CC. Yep, I had the same adventure with libedit and was very 
confused for a moment until we dug into it and realised: 
https://bugs.gentoo.org/870001.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137043/new/

https://reviews.llvm.org/D137043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136283: [clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

2022-10-19 Thread Sam James via Phabricator via cfe-commits
thesamesam created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
thesamesam requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Previously build system generated single huge CompletionModel.cpp with size 
about 6.5 MiB.
Compilation + assembling on PPC32 with GCC + GNU assembler resulted in 22010 
errors:

  {standard input}: Assembler messages:
  {standard input}:181155: Error: operand out of range (0x8164 is 
not between 0x8000 and 0x7fff)
  {standard input}:181192: Error: operand out of range (0x816c is 
not between 0x8000 and 0x7fff)
  {standard input}:181211: Error: operand out of range (0x8080 is 
not between 0x8000 and 0x7fff)
  {standard input}:181220: Error: operand out of range (0x81b8 is 
not between 0x8000 and 0x7fff)
  {standard input}:181225: Error: operand out of range (0x8168 is 
not between 0x8000 and 0x7fff)
  [...]

Separate compilation + assembling of given portion of code avoids exceeding
capabilities of compiler / assembler.

Bug: https://bugs.gentoo.org/829602
Thanks-to: Arfrever Frehtes Taifersar Arahesis 
Tested-by: erhar...@mailbox.org 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136283

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/quality/CompletionModel.cmake
  clang-tools-extra/clangd/quality/CompletionModelCodegen.py
  clang-tools-extra/clangd/unittests/CMakeLists.txt

Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -21,6 +21,7 @@
 
 include(${CMAKE_CURRENT_SOURCE_DIR}/../quality/CompletionModel.cmake)
 gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model DecisionForestRuntimeTest ::ns1::ns2::test::Example)
+file(GLOB DecisionForestRuntimeTest_files "${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest/DecisionForestRuntimeTest*.cpp")
 
 add_custom_target(ClangdUnitTests)
 add_unittest(ClangdUnitTests ClangdTests
@@ -95,7 +96,7 @@
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
-  ${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest.cpp
+  ${DecisionForestRuntimeTest_files}
 
   support/CancellationTests.cpp
   support/ContextTests.cpp
@@ -134,9 +135,9 @@
   $
   )
 
-# Include generated ComletionModel headers.
+# Include generated Completion Model header.
 target_include_directories(ClangdTests PUBLIC
-  $
+  $
 )
 
 clang_target_link_libraries(ClangdTests
Index: clang-tools-extra/clangd/quality/CompletionModelCodegen.py
===
--- clang-tools-extra/clangd/quality/CompletionModelCodegen.py
+++ clang-tools-extra/clangd/quality/CompletionModelCodegen.py
@@ -1,13 +1,15 @@
 """Code generator for Code Completion Model Inference.
 
 Tool runs on the Decision Forest model defined in {model} directory.
-It generates two files: {output_dir}/{filename}.h and {output_dir}/{filename}.cpp
-The generated files defines the Example class named {cpp_class} having all the features as class members.
+It generates files: {output_dir}/{filename}.h, {output_dir}/{filename}.cpp,
+and {output_dir}/{filename}{number}.cpp for each Decision Tree.
+The generated files define the Example class named {cpp_class} having all the features as class members.
 The generated runtime provides an `Evaluate` function which can be used to score a code completion candidate.
 """
 
 import argparse
 import json
+import os
 import struct
 
 
@@ -194,34 +196,36 @@
 `float Evaluate(const {Example}&)` function. This function can be 
 used to score an Example."""
 
-code = ""
+functions_codes = {}
 
 # Generate evaluation function of each tree.
-code += "namespace {\n"
 tree_num = 0
 for tree_json in forest_json:
-code += "LLVM_ATTRIBUTE_NOINLINE float EvaluateTree%d(const %s& E) {\n" % (tree_num, cpp_class.name)
-code += "  " + \
+functions_codes[f"{tree_num}"] = "LLVM_ATTRIBUTE_NOINLINE float EvaluateTree%d(const %s& E) {\n" % (tree_num, cpp_class.name)
+functions_codes[f"{tree_num}"] += "  " + \
 "\n  ".join(
 tree(tree_json, tree_num=tree_num, node_num=0)[0]) + "\n"
-code += "}\n\n"
+functions_codes[f"{tree_num}"] += "}\n"
 tree_num += 1
-code += "} // namespace\n\n"
 
 # Combine the scores of all trees in the final function.
 # MSAN will timeout if these functions are inlined.
-code += "float Evaluate(const %s& E) {\n" % cpp_class.name
-code += "  float Score = 0;\n"
+final_function_code = ""
 for tree_num in range(len(forest_json)):
-   

[PATCH] D136282: [clang] [CMake] Link libclangBasic against libatomic when necessary.

2022-10-19 Thread Sam James via Phabricator via cfe-commits
thesamesam created this revision.
thesamesam added reviewers: mgorny, MaskRay.
Herald added a subscriber: StephenFan.
Herald added a project: All.
thesamesam requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is necessary at least on PPC32.

Depends on D136280 .

Bug: https://bugs.gentoo.org/874024
Thanks-to: Arfrever Frehtes Taifersar Arahesis 
Tested-by: erhar...@mailbox.org 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136282

Files:
  clang/CMakeLists.txt
  clang/lib/Basic/CMakeLists.txt


Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -111,3 +111,7 @@
   omp_gen
   )
 
+target_link_libraries(clangBasic
+  PRIVATE
+  ${LLVM_ATOMIC_LIB}
+)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -57,6 +57,7 @@
   include(TableGen)
   include(HandleLLVMOptions)
   include(VersionFromVCS)
+  include(CheckAtomic)
   include(GetErrcMessages)
   include(LLVMDistributionSupport)
 


Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -111,3 +111,7 @@
   omp_gen
   )
 
+target_link_libraries(clangBasic
+  PRIVATE
+  ${LLVM_ATOMIC_LIB}
+)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -57,6 +57,7 @@
   include(TableGen)
   include(HandleLLVMOptions)
   include(VersionFromVCS)
+  include(CheckAtomic)
   include(GetErrcMessages)
   include(LLVMDistributionSupport)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-10-18 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D134337#3865905 , @tstellar wrote:

> In D134337#3865753 , @mgorny wrote:
>
>> In D134337#3865541 , @tstellar 
>> wrote:
>>
>>> I know I'm a little late here, but having a default config file that's 
>>> always loaded makes triaging issues much harder, because now every time 
>>> someone files a bug, we need to ask for the contents of their config 
>>> directory.
>>
>> I don't see that as much worse than asking them for the values of 
>> `*DEFAULT*` CMake options or whether they have patched their clang because 
>> the driver code makes so many specific assumptions that it didn't work for 
>> them. At least `clang -v` tells us whether they are actually using any 
>> configs.
>
> At least as a distro maintainer, I know the CMake options used for the 
> packages I'm trying to support.
>
> I was also wondering, would it be possible to move some of the targets 
> specific driver code out of the codebase and into config files that are 
> shipped with clang?

We covered the need for this, I feel, quite extensively in the linked 
discussions - especially wrt the Clang 15/16 -Werror changes.

As for bug reports: just ask people to use --no-default-config if something 
looks dodgy?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134337/new/

https://reviews.llvm.org/D134337

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2022-10-13 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

My objection based on config support is no longer relevant, as it's been solved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133375/new/

https://reviews.llvm.org/D133375

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-10-12 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D129755#3853809 , @aaronpuchert 
wrote:

> @aaron.ballman, would like some feedback on the release notes. Should I 
> additionally write something under "Potentially Breaking Changes", or is it 
> enough to mention this under "Improvements to Clang's diagnostics"? Though I 
> guess we could also add this later on if we get more complaints that this 
> breaks things.

I think it's sufficient in "Improvements to Clang's diagnostics". We're not 
looking to document every externally observable change in "Potentially Breaking 
Changes", IMO. Thanks for asking. We could definitely shift it later if needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129755/new/

https://reviews.llvm.org/D129755

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D135701: [clang] Do not override libclang.so's SOVERSION if CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

2022-10-11 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

It fixes a real-life issue I hit with Doxygen, as you know, so I'm happy with 
this - and I don't see a reason to treat libclang specially. I'll wait to see 
what any other reviewers say first though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135701/new/

https://reviews.llvm.org/D135701

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D135545: [clang] Mention -Werror changes revived for Clang 16

2022-10-09 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

I could use identical phrasing to `-Wincompatible-function-pointer-types` below 
(reference how to downgrade the error) but I don't think there's a need.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135545/new/

https://reviews.llvm.org/D135545

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D135545: [clang] Mention -Werror changes revived for Clang 16

2022-10-09 Thread Sam James via Phabricator via cfe-commits
thesamesam created this revision.
thesamesam added reviewers: aaron.ballman, mgorny, MaskRay.
thesamesam added a project: clang.
Herald added a subscriber: StephenFan.
Herald added a project: All.
thesamesam requested review of this revision.
Herald added a subscriber: cfe-commits.

-Wimplicit-function-declaration and -Wimplicit-int become errors
by default in Clang 16 (originally in 15, but we reverted it in 15.0.1).

Mention it in the release notes like we did originally for Clang 15.

See 
https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213
 for more context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135545

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -61,6 +61,17 @@
   into an error-only diagnostic in the next Clang release. Fixes
   `Issue 50055 `_.
 
+- The ``-Wimplicit-function-declaration`` and ``-Wimplicit-int`` warnings
+  now default to an error in C99, C11, and C17. As of C2x,
+  support for implicit function declarations and implicit int has been removed,
+  and the warning options will have no effect. Specifying ``-Wimplicit-int`` in
+  C89 mode will now issue warnings instead of being a noop.
+
+  **NOTE**: We recommend that projects using configure scripts verify that the
+  results do not change before/after setting
+  ``-Werror=implicit-function-declarations`` or ``-Wimplicit-int`` to avoid
+  incompatibility with Clang 16.
+
 - ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
   language modes. It may be downgraded to a warning with
   ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -61,6 +61,17 @@
   into an error-only diagnostic in the next Clang release. Fixes
   `Issue 50055 `_.
 
+- The ``-Wimplicit-function-declaration`` and ``-Wimplicit-int`` warnings
+  now default to an error in C99, C11, and C17. As of C2x,
+  support for implicit function declarations and implicit int has been removed,
+  and the warning options will have no effect. Specifying ``-Wimplicit-int`` in
+  C89 mode will now issue warnings instead of being a noop.
+
+  **NOTE**: We recommend that projects using configure scripts verify that the
+  results do not change before/after setting
+  ``-Werror=implicit-function-declarations`` or ``-Wimplicit-int`` to avoid
+  incompatibility with Clang 16.
+
 - ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
   language modes. It may be downgraded to a warning with
   ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2022-09-13 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D133375#3774293 , @aaron.ballman 
wrote:

> So the basic idea here is that the default can be specified by a 
> configuration file and thus we don't need any configure-time variable for it? 
> But then why do we need other configuration macros like default stdlib or 
> default linker? Mostly trying to understand what the rule of thumb is for 
> when something should be a configure macro and when something should be left 
> to a configuration file instead.

Also, until https://reviews.llvm.org/D109621 or similar is resolved, I don't 
feel like the actual need is addressed. Forcing distributions to use a prefixed 
Clang binary feels like a hack.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133375/new/

https://reviews.llvm.org/D133375

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

2022-09-13 Thread Sam James via Phabricator via cfe-commits
thesamesam added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:52
+  into an error-only diagnostic in the next Clang release. Fixes
+  `Issue 50055: `_.
+- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C

Reflecting on this a bit, do wonder about explicitly calling out configure 
scripts, as I suspect it's something a lot of people may not even think of.

"The LLVM team recommends that projects using configure scripts verify the 
results do not change before/after setting 
`-Werror=implicit-function-declarations` (repeat for others) to avoid 
incompatibility with Clang 16."


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133771/new/

https://reviews.llvm.org/D133771

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

2022-09-13 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133771/new/

https://reviews.llvm.org/D133771

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

2022-02-24 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

Been running with this for some time now (before we added the option). We 
should address the tests but they look sorted now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D116843: [Driver][Gnu] -r: imply -nostdlib like GCC

2022-01-12 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

Looks good, although the documentation for `-r` and other driver options could 
be better (not your fault though).

From https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html:

  -r
  
  Produce a relocatable object as output. This is also known as partial 
linking.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116843/new/

https://reviews.llvm.org/D116843

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE to emulate GCC --enable-default-pie

2021-12-01 Thread Sam James via Phabricator via cfe-commits
thesamesam accepted this revision.
thesamesam added a comment.
This revision is now accepted and ready to land.

This is working well here on Gentoo.

This brings some long-desired feature parity to Clang which we've been wanting 
downstream: GCC has had this option for quite a few years (after pushing from 
various downstreams, including us) and this ends up biting folks who want to 
try migrate to Clang as their system compiler.

The change is rather conservative given we're defaulting to PIE being off for 
now (and this is the same as GCC anyway).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113372/new/

https://reviews.llvm.org/D113372

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits