[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Yeah, okay, this patch makes sense now that I've seen a clash with python's 
ImportError  . I've checked 
lldb c++ files and `ImportError` is not used. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125340: [clang][NFC][AST] rename the ImportError to ASTImportError

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

LLDB should be at least compilable after the change. If it has an internal 
`ImportError` we must not change it. If it uses the renamed class we should 
rename it, or make an "using" alias and no rename (but not this is the best 
solution).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125340

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

[PATCH] D125451: [clang-format] Handle comments below r_brace in RemoveBracesLLVM

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: curdeius, HazardyKnusperkeks, MyDeveloperDay.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If a closing brace is followed by a non-trailing comment, the newline before 
the closing brace must also be removed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125451

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25348,6 +25348,20 @@
"}",
Style);
 
+  verifyFormat("if (a)\n"
+   "  foo();\n"
+   "// comment\n"
+   "else\n"
+   "  bar();",
+   "if (a) {\n"
+   "  foo();\n"
+   "}\n"
+   "// comment\n"
+   "else {\n"
+   "  bar();\n"
+   "}",
+   Style);
+
   verifyFormat("if (a) {\n"
"Label:\n"
"}",
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1898,8 +1898,9 @@
 assert(Token->isOneOf(tok::l_brace, tok::r_brace));
 assert(Token->Next || Token == Line->Last);
 const auto Start =
-Token == Line->Last || (Token->Next->is(tok::kw_else) &&
-Token->Next->NewlinesBefore > 0)
+Token == Line->Last ||
+(Token->Next->isOneOf(tok::kw_else, tok::comment) &&
+ Token->Next->NewlinesBefore > 0)
 ? Token->WhitespaceRange.getBegin()
 : Token->Tok.getLocation();
 const auto Range =


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25348,6 +25348,20 @@
"}",
Style);
 
+  verifyFormat("if (a)\n"
+   "  foo();\n"
+   "// comment\n"
+   "else\n"
+   "  bar();",
+   "if (a) {\n"
+   "  foo();\n"
+   "}\n"
+   "// comment\n"
+   "else {\n"
+   "  bar();\n"
+   "}",
+   Style);
+
   verifyFormat("if (a) {\n"
"Label:\n"
"}",
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1898,8 +1898,9 @@
 assert(Token->isOneOf(tok::l_brace, tok::r_brace));
 assert(Token->Next || Token == Line->Last);
 const auto Start =
-Token == Line->Last || (Token->Next->is(tok::kw_else) &&
-Token->Next->NewlinesBefore > 0)
+Token == Line->Last ||
+(Token->Next->isOneOf(tok::kw_else, tok::comment) &&
+ Token->Next->NewlinesBefore > 0)
 ? Token->WhitespaceRange.getBegin()
 : Token->Tok.getLocation();
 const auto Range =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp:46
+ConstraintManager::ProgramStatePair
+ConstraintManager::assumeDual(ProgramStateRef State, DefinedSVal Cond) {
+  ProgramStateRef StTrue = assume(State, Cond, true);

TODO We should have a very similar implementation for 
`assumeInclusiveRangeDual`! (Not that high prio, that is used only by the 
BoolAssignmentChecker).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124674

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

[PATCH] D125360: [analyzer] Add taint to the BoolAssignmentChecker

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125360

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

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hi,

I noticed when compiling with gcc 9.3.0 that we get a bunch of new warnings 
with this patch:

  [1/351] Building CXX object 
tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/MicrosoftCXXABI.cpp.o
  ../../clang/lib/AST/MicrosoftCXXABI.cpp:57:12: warning: 'virtual unsigned int 
{anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
clang::VarDecl*, unsigned int)' was hidden [-Woverloaded-virtual]
 57 |   unsigned getManglingNumber(const VarDecl *VD,
|^
  ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
 80 |   unsigned getManglingNumber(const TagDecl *TD,
|^
  ../../clang/lib/AST/MicrosoftCXXABI.cpp:46:12: warning: 'virtual unsigned int 
{anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
clang::BlockDecl*)' was hidden [-Woverloaded-virtual]
 46 |   unsigned getManglingNumber(const BlockDecl *BD) override {
|^
  ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
 80 |   unsigned getManglingNumber(const TagDecl *TD,
|^
  ../../clang/lib/AST/MicrosoftCXXABI.cpp:42:12: warning: 'virtual unsigned int 
{anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
clang::CXXMethodDecl*)' was hidden [-Woverloaded-virtual]
 42 |   unsigned getManglingNumber(const CXXMethodDecl *CallOperator) 
override {
|^
  ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
 80 |   unsigned getManglingNumber(const TagDecl *TD,
|^

No idea if it's important or if gcc is overly picky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122734

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:87-88
 const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
-PP->addPPCallbacks(
-::std::make_unique(*this, getLangOpts()));
+  PP->addPPCallbacks(::std::make_unique(
+  *this, getLangOpts(), PP->getSourceManager()));
+}

steakhal wrote:
> whisperity wrote:
> > (🔮: Same here. Is this state that might keep a dangling reference if 
> > multiple TUs are consumed in sequence?)
> How can I acquire the right `SourceManager` if not like this?
> I haven't found any alternative.
> What do you suggest?
Is the `PP` a different variable when a new translation unit is consumed by the 
same check instance? If so, then there is no problem. A simple debug print 
`llvm::errs() << PP << '\n';`, recompile, re-run with `clang-tidy a.cpp b.cpp` 
should show how the infrastructure behaves. I do not know the exact behaviour 
of the preprocessor layer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:135
 {"float.h", "cfloat"},
 {"limits.h", "climits"},
 {"locale.h", "clocale"},

steakhal wrote:
> whisperity wrote:
> > POSIX defines a "global" include `limits.h` which gives you the definition 
> > of macros like `PATH_MAX`. Will such code keep on working if the include is 
> > turned into `climits`?
> IDK.
Did an investigation on Ubuntu with G++ 7.5. So it turns out that the GNU G++ 
standard library implementation, on Linux, is implemented in a way that it will 
include the POSIX-specific `limits.h` into the C Standard `limits.h` and thus 
also `climits`.

And
> Out of these projects 
> llvm-project,**contour**,codechecker,**qtbase**,protobuf,bitcoin,xerces,libwebm,tinyxml2,postgres,ffmpeg,sqlite,openssl,vim,twin,curl,**tmux**,memcached,
>  it suppressed 5 reports

Well, Contour, tmux and Qt (and perhaps even LLVM's OS-specific support lib) 
//SHOULD// be heavily reliant on interfacing with POSIX stuff when compiled 
against Linux, so I guess we can say this transformation is not dangerous.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: libcxx/CMakeLists.txt:250
+
+set(LIBCXX_SUPPORTED_ABI_LIBRARIES none libcxxabi system-libcxxabi libcxxrt 
libstdc++ libsupc++ vcruntime)
+set(LIBCXX_CXX_ABI "${LIBCXX_DEFAULT_ABI_LIBRARY}" CACHE STRING "Specify C++ 
ABI library to use. Supported values are ${LIBCXX_SUPPORTED_ABI_LIBRARIES}.")

All of these values refer to system ABI libraries except for libcxxabi where 
`libcxxabi is the in-tree one and `system-libcxxabi` is the system one. From a 
consistency perspective, I think it'd be better for `libcxxabi` to also refer 
to a system ABI library, and then use a different name for the in-tree one, 
perhaps `default`?



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:76-77
+
+  add_library(libcxx-abi-shared SHARED IMPORTED GLOBAL)
+  target_link_libraries(libcxx-abi-shared INTERFACE libcxx-abi-headers)
+  import_shared_library(libcxx-abi-shared "${LIBCXX_CXX_ABI_LIBRARY_PATH}" 
stdc++)

I'd consider moving these two lines into `import_shared_library` to further 
reduce duplication.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:80-81
+
+  add_library(libcxx-abi-static STATIC IMPORTED GLOBAL)
+  target_link_libraries(libcxx-abi-static INTERFACE libcxx-abi-headers)
+  import_shared_library(libcxx-abi-static "${LIBCXX_CXX_ABI_LIBRARY_PATH}" 
stdc++)

The same here, I'd consider moving these two lines into `import_static_library`.



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:82
+  target_link_libraries(libcxx-abi-static INTERFACE libcxx-abi-headers)
+  import_shared_library(libcxx-abi-static "${LIBCXX_CXX_ABI_LIBRARY_PATH}" 
stdc++)
+

I think this should be `static`?



Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:86
+elseif ("${LIBCXX_CXX_ABI}" STREQUAL "libsupc++")
+add_library(libcxx-abi-headers INTERFACE)
+  import_private_headers(libcxx-abi-headers "${LIBCXX_CXX_ABI_INCLUDE_PATHS}"

The indentation here is off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120727

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

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Nice work! Could you pleas add some lit tests that describe an errno related 
bugreport for a standard lib function?




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:381
 
+  class ErrnoConstraintKind {
+  public:

This class serves as a base class, it is not really a `Kind` class which is 
usually just an enum.
I'd prefer to call this `ErrnoConstraintBase` or `BasicErrnoConstraint`.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:383
+  public:
+virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+  const Summary &Summary,

This class is a polymorphic base class, since we have this virtual function 
here. Would make sense to make destructor virtual as well, even though delete 
is not called explicitly on the base class (as I see).



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:397
+
+  class SingleValueErrnoConstraint : public ErrnoConstraintKind {
+uint64_t const ErrnoValue;

Please document the subclasses.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:603
 
-Summary &Case(ConstraintSet &&CS, StringRef Note = "") {
-  Cases.push_back(SummaryCase(std::move(CS), Note));
+Summary &Case(ConstraintSet &&CS, const ErrnoConstraintKind &ErrnoC,
+  StringRef Note = "") {

Would it make sense to have a `ErrnoIrrelevant` as the default value for 
`ErrnoC`?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:719-722
+  NoErrnoConstraint ErrnoIrrelevant;
+  SuccessErrnoConstraint ErrnoMustNotBeChecked;
+  ZeroRelatedErrnoConstraint ErrnoNEZeroIrrelevant{
+  clang::BinaryOperatorKind::BO_NE, errno_modeling::Errno_Irrelevant};

Could you please add some more documentation to these variables? And they could 
be `const`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125400

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

[PATCH] D121733: Clean pathnames in FileManager.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:24
+  static auto *Mappings =
+  new std::array, 645>{{
+  {"algorithm", ""},

ppluzhnikov wrote:
> ilya-biryukov wrote:
> > Don't specify sizes of arrays explicitly. This is error prone.
> std::array requires size.
> 
> I could use std::vector instead, at the cost of an extra allocation.
Use raw arrays.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:546
+  {"include/wordexp.h", ""},
+  {"include/x86intrin.h", ""},
+  {"include/xlocale.h", ""},

ppluzhnikov wrote:
> ilya-biryukov wrote:
> > Why do we change the order of elements here?
> > Please revert, this increases the diff and is not relevant to the actual 
> > change.
> Note that the elements are inserted into a map
> (after commit b3a991df3cd6a; used to be a vector before).
> 
> Also note that there are duplicates, e.g.
> 
> {"bits/typesizes.h", ""},
> {"bits/typesizes.h", ""},
> 
> which can't work as intended / is already broken.
> 
> Sorting helps to find these duplicates.
> 
This refactoring makes sense, but please split this into a separate change.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:670
+  }};
+  auto *HeaderMapping = new llvm::StringMap(Mappings->size());
+

ppluzhnikov wrote:
> ilya-biryukov wrote:
> > This line introduces a memory leak.
> > Notice how the previous version had a `static` variable.
> No, it does not. This function is called only once to initialize a static 
> variable: 
> 
> static const auto *SystemHeaderMap = GetHeaderMapping();
>  
There is no guarantee someone won't run this again by mistake in the future 
revisions.
Make this function leak-free, don't rely on global invariants.

It's easy to do with a lambda trick from the next comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

[Gabor Marton via Phabricator via cfe-commits]

martong marked 3 inline comments as done.
martong added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116
 
+  const bool Foreign = false; // From CTU.
+

xazax.hun wrote:
> martong wrote:
> > martong wrote:
> > > xazax.hun wrote:
> > > > I feel that we use different terms for the imported declarations. 
> > > > Sometimes we call them `new`, sometimes `imported`, sometimes 
> > > > `foreign`. In case all of these means the same thing, it would be nice 
> > > > to standardize on a single way of naming. If there is a subtle 
> > > > difference between them, let's document that in a comment. It would be 
> > > > nice if we did not need the comment after the declaration but it would 
> > > > be obvious from the variable name.
> > > Yes, I agree that this should deserver some more explanation. Maybe right 
> > > above this declaration?
> > > 
> > > So, `new` means that a declaration is **created** newly by the 
> > > ASTImporter.
> > > `imported` means it has been imported, but not necessarily `new`. Think 
> > > about this case, we import `foo`'s definition.
> > > ```
> > > // to.cpp
> > > void bar() {} // from a.h
> > > // from.cpp
> > > void bar() {} // from a.h
> > > void foo() {
> > >   bar();
> > > }
> > > ```
> > > Then `foo` will be `new` and `imported`, `bar` will be `imported` and not 
> > > `new`.  
> > > `foreign` basically means `imported` and `new`.
> > I've just added an explanatory comment for this field.
> Foreign means new and imported. But is there a way for a declaration to be 
> new and not to be imported? If no, in that case it feels like new and foreign 
> are actually the same and we should standardize on a single name.
Yeah, you are right, `new` and `foreign` are the same in this sense. However, I 
think the term `foreign` is more expressive in the sense that is suggests that 
the definition is coming from another translation unit.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446
+}
+const bool BState = State->contains(D);
+if (!BState) { // This is the first time we see this foreign function.

xazax.hun wrote:
> xazax.hun wrote:
> > martong wrote:
> > > xazax.hun wrote:
> > > > So if we see the same foreign function called in multiple contexts, we 
> > > > will only queue one of the contexts for the CTU. Is this the intended 
> > > > design? 
> > > > 
> > > > So if I see:
> > > > ```
> > > > foreign(true);
> > > > foreign(false);
> > > > ```
> > > > 
> > > > The new CTU will only evaluate `foreign(true)` but not 
> > > > `foreign(false)`. 
> > > This is intentional.
> > > ```
> > > foreign(true);
> > > foreign(false);
> > > ```
> > > The new CTU will evaluate the following paths in the exploded graph:
> > > ```
> > > foreign(true); // conservative evaluated
> > > foreign(false); // conservative evaluated
> > > foreign(true); // inlined
> > > foreign(false); // inlined
> > > ```
> > > The point is to keep bifurcation to a minimum and avoid the exponential 
> > > blow up.
> > > So, we will never have a path like this:
> > > ```
> > > foreign(true); // conservative evaluated
> > > foreign(false); // inlined
> > > ```
> > > 
> > > Actually, this is the same strategy that we use during the dynamic 
> > > dispatch of C++ virtual calls. See `DynamicDispatchBifurcationMap`.
> > > 
> > > The conservative evaluation happens in the first phase, the inlining in 
> > > the second phase (assuming the phase1 inlining option is set to none).
> > > The new CTU will evaluate the following paths in the exploded graph:
> > > ```
> > > foreign(true); // conservative evaluated
> > > foreign(false); // conservative evaluated
> > > foreign(true); // inlined
> > > foreign(false); // inlined
> > > ```
> > 
> > When we encounter `foreign(true)`, we would add the decl to 
> > `CTUDispatchBifurcationSet`. So the second time we see a call to the 
> > function `foreign(false);`, we will just do the conservative evaluation and 
> > will not add the call to the CTU worklist. So how will `foreign(false);` be 
> > inlined in the second pass? Do I miss something? 
> > 
> Oh, I think I now understand. Do you expect `foreign(false);` to be inlined 
> after we return from `foreign(true);` in the second pass? Sorry for the 
> confusion, in that case it looks good to me.
> Oh, I think I now understand. Do you expect `foreign(false);` to be inlined 
> after we return from `foreign(true);` in the second pass?

Yes, that's right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D124860: [clang][AArch64][SVE] Implicit conversions for vector-scalar operations

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:10586
+  (!LHSBuiltinTy && !LHSBuiltinTy->isVLSTBuiltinType() &&
+   !LHSType->isRealType())) {
+Diag(Loc, diag::err_typecheck_vector_not_convertable_non_scalar)

`!Ptr && [Deref Ptr]` looks suspicious. Also, if I insert an abort in this path 
and run check-clang I don't see any failing tests?

It might help to name `LHSBuiltinTy && LHSBuiltinTy->isVLSTBuiltinType()` 
`IsLHSVector` (likewise for RHS) or similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124860

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

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The bugreports look promising. However, I think we desperately need a note that 
describes which function has set the `errno_modeling` state. Below I'd expect 
the following notes for the highlighted function call.

- assuming return value of `mkdir` is in range [0, INT_MAX]
- errno is not set

F23040017: image.png 

I suppose, you'd need to add some changes for these both in the ErrnoChecker 
and here as well.




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:435
+  assert(Cond);
+  State = State->assume(*Cond, true);
+  return errno_modeling::setErrnoValue(State, C.getLocationContext(),

Please check if `State` can be nullptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125400

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Balázs Benics via Phabricator via cfe-commits]

steakhal marked 6 inline comments as done.
steakhal added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:87-88
 const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
-PP->addPPCallbacks(
-::std::make_unique(*this, getLangOpts()));
+  PP->addPPCallbacks(::std::make_unique(
+  *this, getLangOpts(), PP->getSourceManager()));
+}

whisperity wrote:
> steakhal wrote:
> > whisperity wrote:
> > > (🔮: Same here. Is this state that might keep a dangling reference if 
> > > multiple TUs are consumed in sequence?)
> > How can I acquire the right `SourceManager` if not like this?
> > I haven't found any alternative.
> > What do you suggest?
> Is the `PP` a different variable when a new translation unit is consumed by 
> the same check instance? If so, then there is no problem. A simple debug 
> print `llvm::errs() << PP << '\n';`, recompile, re-run with `clang-tidy a.cpp 
> b.cpp` should show how the infrastructure behaves. I do not know the exact 
> behaviour of the preprocessor layer.
Okay, I've checked this. The lifetimes checks out. Thanks!
```
$ ./build/debug/bin/clang-tidy a.cpp b.cpp  
-checks="-*,modernize-deprecated-headers"
DeprecatedHeadersCheck::registerPPCallbacks() acquires SM 0x27eb9e0
ctor IncludeModernizePPCallbacks acquires SM 0x27eb9e0
IncludeModernizePPCallbacks::InclusionDirective inserts to 
IncludesToBeProcessed using SM 0x27eb9e0
IncludeModernizePPCallbacks::InclusionDirective inserts to 
IncludesToBeProcessed using SM 0x27eb9e0
DeprecatedHeadersCheck::check() emits IncludesToBeProcessed using SM 0x27eb9e0
onEndOfTranslationUnit cleares IncludesToBeProcessed
1 warning generated.
dtor IncludeModernizePPCallbacks releases SM 0x27eb9e0
DeprecatedHeadersCheck::registerPPCallbacks() acquires SM 0x27eb9e0
ctor IncludeModernizePPCallbacks acquires SM 0x27eb9e0
IncludeModernizePPCallbacks::InclusionDirective inserts to 
IncludesToBeProcessed using SM 0x27eb9e0
IncludeModernizePPCallbacks::InclusionDirective inserts to 
IncludesToBeProcessed using SM 0x27eb9e0
DeprecatedHeadersCheck::check() emits IncludesToBeProcessed using SM 0x27eb9e0
onEndOfTranslationUnit cleares IncludesToBeProcessed
2 warnings generated.
dtor IncludeModernizePPCallbacks releases SM 0x27eb9e0
a.cpp:2:10: warning: inclusion of deprecated C++ header 'assert.h'; consider 
using 'cassert' instead [modernize-deprecated-headers]
#include 
 ^~
 
b.cpp:2:10: warning: inclusion of deprecated C++ header 'assert.h'; consider 
using 'cassert' instead [modernize-deprecated-headers]
#include 
 ^~
 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/include/clang/AST/ASTImporterSharedState.h:83
+
+  void setNewDecl(Decl *ToD) { NewDecls.insert(ToD); }
 };

(The naming of this function feels a bit odd. `markAsNewDecl` or just 
`markNewDecl`?)



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:413
+unsigned, CTUMaxNodesMultiplier, "ctu-max-nodes-mul",
+"We count the nodes for a normal single tu analysis. We multiply that "
+"number with this config value then we divide the result by 100 to get "





Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:413-419
+"We count the nodes for a normal single tu analysis. We multiply that "
+"number with this config value then we divide the result by 100 to get "
+"the maximum number of nodes that the analyzer can generate while "
+"exploring a top level function in CTU mode (for each exploded graph)."
+"For example, 100 means that CTU will explore maximum as much nodes that "
+"we explored during the single tu mode, 200 means it will explore twice as 
"
+"much, 50 means it will explore maximum 50% more.", 100)

whisperity wrote:
> 
Couldn't this description here be simplified to say something along the lines 
of //"the percentage of single-TU analysed nodes that the CTU analysis is 
allowed to visit"//? Is the calculation method needed from the user's 
perspective? The examples talk about percentage too.



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:141
 
+enum class CTUPhase1InliningKind { None, Small, All };
+

Is this configuration inherent to the static analyser, and not the //CrossTU// 
library?



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:141
 
+enum class CTUPhase1InliningKind { None, Small, All };
+

whisperity wrote:
> Is this configuration inherent to the static analyser, and not the 
> //CrossTU// library?
(Documentation for the options are missing.)



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:156
   llvm::PointerUnion Origin;
+  mutable Optional Foreign; // From CTU.
 





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:814-816
+  /// Returns true if the CTU analysis is running its first phase.
+  /// Returns true in single TU (non-CTU) mode!
+  bool isCTUInFirtstPhase() { return Engine.getCTUWorkList(); }

How and why is this needed? Could you call it `isSingleTUOr1stPhaseCTU` instead?

Rather, if this is used as a distinction input in conditionals, could you 
invert the branches and have a function `isSecondPhaseCTU`, and do the inverted 
logic where this function is consumed?



Comment at: 
clang/test/Analysis/Inputs/ctu-onego-existingdef-other.cpp.externalDefMap.ast-dump.txt:1
+11:c:@F@other# ctu-onego-other.cpp.ast

Why is there only 1 symbol in this file, when the file above contains two 
function definitions?



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:22
 //
+// FIXME On-demand ctu should be tested in the same file that we have for the
+// PCH version, but with a different verify prefix (e.g. -verfiy=on-demand-ctu)





Comment at: clang/test/Analysis/ctu-on-demand-parsing.cpp:33
+
+// FIXME On-demand ctu should be tested in the same file that we have for the
+// PCH version, but with a different verify prefix (e.g. -verfiy=on-demand-ctu)





Comment at: clang/test/Analysis/ctu-onego-toplevel.cpp:24
+// RUN:   -verify=ctu %s 2>&1 | FileCheck %s
+//
+// CallGraph: c->b

Are the lines below related to the execution of the command above? If not, 
could you please break the comment?



Comment at: clang/test/Analysis/ctu-onego-toplevel.cpp:28
+// Note that `other` calls into `b` but that is not visible in the CallGraph
+// b/c that happens in another TU.
+





Comment at: clang/test/Analysis/ctu-onego-toplevel.cpp:30
+
+// During the onego CTU analysis, we start with c() as top level function.
+// Then we visit b() as non-toplevel during the processing of the FWList, thus

One-go? And what does that refer to? Is "Onego" analysis the one this patch is 
introducing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

LGTM. Some typos inline. Also I think this is a significant enough bug-fix that 
it warrants an entry in the Release Notes under `Changes to existing checks`.

Also, the tests are failing, but it fails on the formatting of the test file 
(???), and not the actual test itself. (Usually such issues would be sent back 
to Phab to appear an a machine-made inline comment, I am not sure what happened 
to that logic.)

  changed files:
  
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp




Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:79-82
+auto Begin = IncludesToBeProcessed.begin();
+auto End = IncludesToBeProcessed.end();
+auto Low = std::lower_bound(Begin, End, ExternCBlockBegin);
+auto Up = std::upper_bound(Low, End, ExternCBlockEnd);

(`llvm/ADT/STLExtras.h` define a range-based `lower_bound` and `upper_bound`)



Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:98
+  // to act on a "TranslationUnit" to acquire the AST where we can walk each
+  // decl and look for `extern "C"` blocks where we wil lsuppress the report we
+  // collected during the preprocessing phase.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

(Also: this is a fix to an issue of your own finding and not something that was 
reported on Bugzilla? Just to make sure we cross-reference and close tickets.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[clang] b6d8c84 - [clang-format] Don't remove braces if a 1-statement body would wrap

[via cfe-commits]

Author: owenca
Date: 2022-05-12T03:53:08-07:00
New Revision: b6d8c84f28103104a5707091f970d80df423b6c9

URL: 
https://github.com/llvm/llvm-project/commit/b6d8c84f28103104a5707091f970d80df423b6c9
DIFF: 
https://github.com/llvm/llvm-project/commit/b6d8c84f28103104a5707091f970d80df423b6c9.diff

LOG: [clang-format] Don't remove braces if a 1-statement body would wrap

Reimplement the RemoveBracesLLVM feature which handles a
single-statement block that would get wrapped.

Fixes #53543.

Differential Revision: https://reviews.llvm.org/D125137

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index f90e8fcb834c7..6ba8edccbb117 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,6 +14,7 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
+#include "TokenAnnotator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -460,6 +461,7 @@ bool UnwrappedLineParser::precededByCommentOrPPDirective() 
const {
   return Previous && Previous->is(tok::comment) &&
  (Previous->IsMultiline || Previous->NewlinesBefore > 0);
 }
+
 /// \brief Parses a level, that is ???.
 /// \param HasOpeningBrace If that level is started by an opening brace.
 /// \param CanContainBracedList If the content can contain (at any level) a
@@ -751,6 +753,50 @@ size_t UnwrappedLineParser::computePPHash() const {
   return h;
 }
 
+// Checks whether \p ParsedLine might fit on a single line. We must clone the
+// tokens of \p ParsedLine before running the token annotator on it so that we
+// can restore them afterward.
+bool UnwrappedLineParser::mightFitOnOneLine(UnwrappedLine &ParsedLine) const {
+  const auto ColumnLimit = Style.ColumnLimit;
+  if (ColumnLimit == 0)
+return true;
+
+  auto &Tokens = ParsedLine.Tokens;
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);
+
+  SmallVector SavedTokens(Tokens.size());
+
+  int Index = 0;
+  for (const auto &Token : Tokens) {
+assert(Token.Tok);
+auto &SavedToken = SavedTokens[Index++];
+SavedToken.Tok = new FormatToken;
+SavedToken.Tok->copyFrom(*Token.Tok);
+SavedToken.Children = std::move(Token.Children);
+  }
+
+  AnnotatedLine Line(ParsedLine);
+  assert(Line.Last == LastToken);
+
+  TokenAnnotator Annotator(Style, Keywords);
+  Annotator.annotate(Line);
+  Annotator.calculateFormattingInformation(Line);
+
+  const int Length = LastToken->TotalLength;
+
+  Index = 0;
+  for (auto &Token : Tokens) {
+const auto &SavedToken = SavedTokens[Index++];
+Token.Tok->copyFrom(*SavedToken.Tok);
+Token.Children = std::move(SavedToken.Children);
+delete SavedToken.Tok;
+  }
+
+  return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
+}
+
 UnwrappedLineParser::IfStmtKind
 UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
 bool MunchSemi, bool UnindentWhitesmithsBraces,
@@ -813,8 +859,11 @@ UnwrappedLineParser::parseBlock(bool MustBeDeclaration, 
unsigned AddLevels,
 const FormatToken *Previous = Tokens->getPreviousToken();
 assert(Previous);
 if (Previous->isNot(tok::r_brace) || Previous->Optional) {
-  Tok->MatchingParen = FormatTok;
-  FormatTok->MatchingParen = Tok;
+  assert(!CurrentLines->empty());
+  if (mightFitOnOneLine(CurrentLines->back())) {
+Tok->MatchingParen = FormatTok;
+FormatTok->MatchingParen = Tok;
+  }
 }
   }
 

diff  --git a/clang/lib/Format/UnwrappedLineParser.h 
b/clang/lib/Format/UnwrappedLineParser.h
index 3334b5bad97b4..aea999586ebe3 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -95,6 +95,7 @@ class UnwrappedLineParser {
   bool parseLevel(bool HasOpeningBrace, bool CanContainBracedList,
   IfStmtKind *IfKind = nullptr,
   TokenType NextLBracesType = TT_Unknown);
+  bool mightFitOnOneLine(UnwrappedLine &Line) const;
   IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 
1u,
 bool MunchSemi = true,
 bool UnindentWhitesmithsBraces = false,

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 8f9c7215d6193..2a764b68b9129 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25365,8 +25365,6 @@ TEST_F(FormatTest, RemoveBraces) {
"}",
Style);
 
-  // FIXME: See https://github.com/llvm/llvm-project/issues/53543.
-#if 0
   Style.ColumnLimit = 65;
 
   verifyFormat("if (condition) {\n"
@@ -25380

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Owen Pan 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 rGb6d8c84f2810: [clang-format] Don't remove braces if a 
1-statement body would wrap (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125137

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25365,8 +25365,6 @@
"}",
Style);
 
-  // FIXME: See https://github.com/llvm/llvm-project/issues/53543.
-#if 0
   Style.ColumnLimit = 65;
 
   verifyFormat("if (condition) {\n"
@@ -25380,6 +25378,15 @@
 
   Style.ColumnLimit = 20;
 
+  verifyFormat("int ab = [](int i) {\n"
+   "  if (i > 0) {\n"
+   "i = 12345678 -\n"
+   "i;\n"
+   "  }\n"
+   "  return i;\n"
+   "};",
+   Style);
+
   verifyFormat("if (a) {\n"
"  b = c + // 1 -\n"
"  d;\n"
@@ -25394,9 +25401,6 @@
"  b = c >= 0 ? d : e;\n"
"}",
Style);
-#endif
-
-  Style.ColumnLimit = 20;
 
   verifyFormat("if (a)\n"
"  b = c > 0 ? d : e;",
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -95,6 +95,7 @@
   bool parseLevel(bool HasOpeningBrace, bool CanContainBracedList,
   IfStmtKind *IfKind = nullptr,
   TokenType NextLBracesType = TT_Unknown);
+  bool mightFitOnOneLine(UnwrappedLine &Line) const;
   IfStmtKind parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u,
 bool MunchSemi = true,
 bool UnindentWhitesmithsBraces = false,
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,6 +14,7 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
+#include "TokenAnnotator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -460,6 +461,7 @@
   return Previous && Previous->is(tok::comment) &&
  (Previous->IsMultiline || Previous->NewlinesBefore > 0);
 }
+
 /// \brief Parses a level, that is ???.
 /// \param HasOpeningBrace If that level is started by an opening brace.
 /// \param CanContainBracedList If the content can contain (at any level) a
@@ -751,6 +753,50 @@
   return h;
 }
 
+// Checks whether \p ParsedLine might fit on a single line. We must clone the
+// tokens of \p ParsedLine before running the token annotator on it so that we
+// can restore them afterward.
+bool UnwrappedLineParser::mightFitOnOneLine(UnwrappedLine &ParsedLine) const {
+  const auto ColumnLimit = Style.ColumnLimit;
+  if (ColumnLimit == 0)
+return true;
+
+  auto &Tokens = ParsedLine.Tokens;
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);
+
+  SmallVector SavedTokens(Tokens.size());
+
+  int Index = 0;
+  for (const auto &Token : Tokens) {
+assert(Token.Tok);
+auto &SavedToken = SavedTokens[Index++];
+SavedToken.Tok = new FormatToken;
+SavedToken.Tok->copyFrom(*Token.Tok);
+SavedToken.Children = std::move(Token.Children);
+  }
+
+  AnnotatedLine Line(ParsedLine);
+  assert(Line.Last == LastToken);
+
+  TokenAnnotator Annotator(Style, Keywords);
+  Annotator.annotate(Line);
+  Annotator.calculateFormattingInformation(Line);
+
+  const int Length = LastToken->TotalLength;
+
+  Index = 0;
+  for (auto &Token : Tokens) {
+const auto &SavedToken = SavedTokens[Index++];
+Token.Tok->copyFrom(*SavedToken.Tok);
+Token.Children = std::move(SavedToken.Children);
+delete SavedToken.Tok;
+  }
+
+  return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
+}
+
 UnwrappedLineParser::IfStmtKind
 UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
 bool MunchSemi, bool UnindentWhitesmithsBraces,
@@ -813,8 +859,11 @@
 const FormatToken *Previous = Tokens->getPreviousToken();
 assert(Previous);
 if (Previous->isNot(tok::r_brace) || Previous->Optional) {
-  Tok->MatchingParen = FormatTok;
-  FormatTok->MatchingParen = Tok;
+  assert(!CurrentLines->empty());
+  if (mightFitOnOneLine(CurrentLines->back())) {
+Tok->MatchingParen = FormatTok;
+FormatTok->MatchingParen = Tok;
+  }
 }
   }
 
__

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Balázs Benics via Phabricator via cfe-commits]

steakhal marked 3 inline comments as done.
steakhal added a comment.

In D125209#3508573 , @whisperity 
wrote:

> LGTM. Some typos inline. Also I think this is a significant enough bug-fix 
> that it warrants an entry in the Release Notes under `Changes to existing 
> checks`.
>
> Also, the tests are failing, but it fails on the formatting of the test file 
> (???), and not the actual test itself. (Usually such issues would be sent 
> back to Phab to appear an a machine-made inline comment, I am not sure what 
> happened to that logic.)
>
>   changed files:
>   
> clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp

Thanks.

In D125209#3508576 , @whisperity 
wrote:

> (Also: this is a fix to an issue of your own finding and not something that 
> was reported on Bugzilla? Just to make sure we cross-reference and close 
> tickets.)

It was an internal ticket.




Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:79-82
+auto Begin = IncludesToBeProcessed.begin();
+auto End = IncludesToBeProcessed.end();
+auto Low = std::lower_bound(Begin, End, ExternCBlockBegin);
+auto Up = std::upper_bound(Low, End, ExternCBlockEnd);

whisperity wrote:
> (`llvm/ADT/STLExtras.h` define a range-based `lower_bound` and `upper_bound`)
Yup, but I'm using a different range.
The first call could be replaced by the `llvm` ranged version, but the second 
starts with `Low` instead of `Begin`.
At that point, I decided to keep the symmetry and use the `std` versions in 
both cases.

Well, I could `llvm::make_range()` and use the `llvm::upper_bound` but yea, no.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 428904.
steakhal marked an inline comment as done.
steakhal added a comment.

- Fix typos.
- Mention this in the release notes.
- `clang-format` the test file as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

Files:
  clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
  
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s modernize-deprecated-headers %t \
+// RUN:   -- -header-filter='.*' -system-headers \
+// RUN:  -extra-arg-before=-isystem%S/Inputs/modernize-deprecated-headers
+
+#define EXTERN_C extern "C"
+
+extern "C++" {
+// We should still have the warnings here.
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
+}
+
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: {{^}}#include {{$}}
+
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]
+
+#include 
+// CHECK-MESSAGES: mylib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+
+namespace wrapping {
+extern "C" {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+}
+} // namespace wrapping
+
+extern "C" {
+namespace wrapped {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+} // namespace wrapped
+}
+
+namespace wrapping {
+extern "C" {
+namespace wrapped {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+} // namespace wrapped
+}
+} // namespace wrapping
+
+EXTERN_C {
+#include   // no-warning
+#include// no-warning
+#include  // no-warning
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
@@ -0,0 +1 @@
+#include "assert.h"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
   ` involving assignments in
   conditions. This fixes `Issue 35853 `_.
 
+- Fixed a false positive in :doc:`modernize-deprecated-headers
+  ` involving including
+  C header files from C++ files wrapped by ``extern "C" { ... }`` blocks.
+  Such includes will be ignored by now.
+
 - Improved :doc:`performance-inefficient-vector-operation
   ` to work when
   the vector is a member of a structure.
Index: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
+++ clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
@@ -15,6 +15,22 @@
 namespace tidy {
 namespace modernize {
 
+namespace detail {
+class IncludeModernizePPCallbacks;
+class ExternCRefutationVisitor;
+struct IncludeMarker {
+  std::string Replacement;
+  StringRef FileName;
+  SourceRange ReplacementRange;
+  std::pair DecomposedDiagLoc;
+};
+bool operator<(const IncludeMarker &LHS, const IncludeMarker &RHS);
+bool operator<(const IncludeMarker &LHS,
+   const std::pair &RHS);
+bool operator<(const std::pair &LHS,
+   const IncludeMarker &RHS);
+} // namespace detail
+
 /// This check replaces deprecated C library headers with their C++ STL
 /// alternatives.
 ///
@@ -41,6 +57,14 @@
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void onEndOfTranslationUnit() override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  friend class detail::IncludeModernizePPCallbacks;
+  friend class detail::ExternCRefutationVisitor;
+  std::vector IncludesToBeProcessed;
 };
 
 } // namespace modernize
Ind

[PATCH] D124946: [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan accepted this revision.
urnathan added a comment.
This revision is now accepted and ready to land.

good to go, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124946

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/AST/CommentCommands.td:93
 
+def N  : InlineCommand<"n", 0>;
+

Could you add a test that shows that the text after \n is not treated as the 
argument?

You could verify the comment AST like here: 
llvm-project/clang/test/AST/ast-dump-comment.cpp



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

Thank you for additional checks. Unfortunatelly, I am not able to perform 
commits for this and next wee (up to 20th of May), so I wonder if you could 
commit this one for me.


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

https://reviews.llvm.org/D124658

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Thanks @aaronpuchert !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In general, I think this approach is defensible. Though I do wonder if we want 
to put an assert in the check functions to assert that the command is what 
you'd expect, so someone calling the check functions incorrectly will get some 
early feedback. WDYT?




Comment at: clang/lib/AST/CommentSema.cpp:666-667
 void Sema::checkDeprecatedCommand(const BlockCommandComment *Command) {
-  if (!Traits.getCommandInfo(Command->getCommandID())->IsDeprecatedCommand)
-return;
-

e.g., turn this into an assert so if someone calls checkDeprectedCommand when 
it's not a deprecated command, they get yelled at. This makes asserts builds 
slightly slower (because we're getting the command ID twice), but I don't think 
the overhead should be all that noticeable in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113793

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Oops, it looks like precommit CI found a relevant issue, can you fix it up?

  Failed Tests (1):
Clang :: Preprocessor/elifdef.c
   
  Testing Time: 790.67s
Skipped  :56
Unsupported  :  1997
Passed   : 87720
Expectedly Failed:   234
Failed   : 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428910.
ken-matsui added a comment.

Fix the failed test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-pp-directive.c
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-pp-directive.c
@@ -0,0 +1,59 @@
+// For C
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=pre-c2x-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+// For C++
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// For C
+// pre-c2x-pedantic-warning@#1 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// For C
+// pre-c2x-pedantic-warning@#2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#2 {{use of a '#elifndef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C++ standards before C++2b}}
+
+#if 0
+#elifdef C
+#endif
+// For C
+// pre-c2x-pedantic-warning@-3 {{use of a '#elifd

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

I fixed it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:421
 
 install(
   FILES ${ppc_wrapper_files}

craig.topper wrote:
> There appear to be two installs of ppc_wrapper_files with different 
> components. Is that intentional?
Ah yes this is indeed intentional. `cuda_wrapper_files` and 
`openmp_wrapper_files` have two install targets as well. The first target is 
part of the "catch-all" `clang-resource-headers`.  The second targets are not 
installed by default (`EXCLUDE_FROM_ALL`). and a distribution build can opt-in 
the relevant set of headers. 

That said, if there are better ways to implement the logic I am all ears. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

You should also come up with a release note for the changes.




Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:431-432
+def err_pp_invalid_directive : Error<
+  "invalid preprocessing directive%select{|, did you mean '#%1'?}0"
+>;
+def warn_pp_invalid_directive : Warning<





Comment at: clang/lib/Lex/PPDirectives.cpp:488-490
+  if (LangOpts.C2x || LangOpts.CPlusPlus2b) {
+Candidates.insert(Candidates.end(), {"elifdef", "elifndef"});
+  }





Comment at: clang/lib/Lex/PPDirectives.cpp:1257
   // If we reached here, the preprocessing token is not valid!
-  Diag(Result, diag::err_pp_invalid_directive);
+  Diag(Result, diag::err_pp_invalid_directive) << 0;
 

I think we should be attempting to suggest a typo for the error case as well 
e.g.,
```
#fi WHATEVER
#endif
```
we currently give no suggestion for that typo, just the error. However, this 
may require a fair amount of changes because of the various edge cases where we 
give better diagnostics than "unknown directive". e.g.,
```
#if WHATEVER // error: unterminated conditional directive
#endfi // no diagnostic
```
so if it looks like covering error cases is going to be involved, I'm fine 
doing it in a follow-up if you'd prefer.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.c:36
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you 
mean '#elifndef'?}}

ken-matsui wrote:
> Here, `#elfindef` is suggested to `#elifdef`, not `#elifndef`, but I believe 
> it will help many users to realize that they have typo'd `#elifndef`, or 
> maybe they might have meant actually `#elifdef`.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125463: [analyzer][NFC] Tighten some of the SValBuilder return types

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, martong, ASDenysPetrov.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Cosmetic change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125463

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -74,8 +74,10 @@
   return UnknownVal();
 }
 
-NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-const llvm::APSInt& rhs, QualType type) {
+nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *lhs,
+  BinaryOperator::Opcode op,
+  const llvm::APSInt &rhs,
+  QualType type) {
   // The Environment ensures we always get a persistent APSInt in
   // BasicValueFactory, so we don't need to get the APSInt from
   // BasicValueFactory again.
@@ -84,23 +86,24 @@
   return nonloc::SymbolVal(SymMgr.getSymIntExpr(lhs, op, rhs, type));
 }
 
-NonLoc SValBuilder::makeNonLoc(const llvm::APSInt& lhs,
-   BinaryOperator::Opcode op, const SymExpr *rhs,
-   QualType type) {
+nonloc::SymbolVal SValBuilder::makeNonLoc(const llvm::APSInt &lhs,
+  BinaryOperator::Opcode op,
+  const SymExpr *rhs, QualType type) {
   assert(rhs);
   assert(!Loc::isLocType(type));
   return nonloc::SymbolVal(SymMgr.getIntSymExpr(lhs, op, rhs, type));
 }
 
-NonLoc SValBuilder::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-   const SymExpr *rhs, QualType type) {
+nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *lhs,
+  BinaryOperator::Opcode op,
+  const SymExpr *rhs, QualType type) {
   assert(lhs && rhs);
   assert(!Loc::isLocType(type));
   return nonloc::SymbolVal(SymMgr.getSymSymExpr(lhs, op, rhs, type));
 }
 
-NonLoc SValBuilder::makeNonLoc(const SymExpr *operand,
-   QualType fromTy, QualType toTy) {
+nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *operand,
+  QualType fromTy, QualType toTy) {
   assert(operand);
   assert(!Loc::isLocType(toTy));
   return nonloc::SymbolVal(SymMgr.getCastSymbol(operand, fromTy, toTy));
@@ -1009,7 +1012,7 @@
   return V;
 }
 
-SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,
+nonloc::SymbolVal SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,
   QualType CastTy) {
   // We use seven conditions to recognize a simplification case.
   // For the clarity let `CastTy` be `C`, SE->getType() - `T`, root type - `R`,
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -106,7 +106,7 @@
   /// \param CastTy -- QualType, which `V` shall be cast to.
   /// \return SVal with simplified cast expression.
   /// \note: Currently only support integral casts.
-  SVal simplifySymbolCast(nonloc::SymbolVal V, QualType CastTy);
+  nonloc::SymbolVal simplifySymbolCast(nonloc::SymbolVal V, QualType CastTy);
 
 public:
   SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
@@ -340,17 +340,19 @@
 return nonloc::LocAsInteger(BasicVals.getPersistentSValWithData(loc, bits));
   }
 
-  NonLoc makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-const llvm::APSInt& rhs, QualType type);
+  nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
+   const llvm::APSInt &rhs, QualType type);
 
-  NonLoc makeNonLoc(const llvm::APSInt& rhs, BinaryOperator::Opcode op,
-const SymExpr *lhs, QualType type);
+  nonloc::SymbolVal makeNonLoc(const llvm::APSInt &rhs,
+   BinaryOperator::Opcode op, const SymExpr *lhs,
+   QualType type);
 
-  NonLoc makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-const SymExpr *rhs, QualType type);
+  nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode o

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, clang-language-wg.
aaron.ballman added a comment.

Thank you for working on this new diagnostic! We don't typically add new 
diagnostics that are off by default unless they're for pedantic diagnostics or 
are reasonably expected to be enabled by default in the future. This is because 
we've had evidence that suggests such diagnostics aren't enabled often enough 
to warrant the maintenance cost for them.

I'm not convinced this diagnostic can be enabled by default. Yes, it prevents 
move semantics, but that's not typically an issue with the correctness of the 
code. IIRC, we decided to put this diagnostic into clang-tidy because we 
thought it would be too chatty in practice, especially on older code bases.

Perhaps there are ways we can improve the diagnostic behavior to allow it to be 
enabled by default though. One possibility would be to look at the return type 
to see if there's a user-provided move constructor (either `= default` or 
explicitly written) and only diagnose if one is found. But I think we'd want 
some evidence that this actually reduces the false positive rate in practice, 
which means trying to compile some large C++ projects (of various ages/code 
quality) to see. Would you be willing to run your patch against some large C++ 
projects to see what kind of true and false positives it generates? From there, 
we can decide whether we need additional heuristics or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125402

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

[clang] a1545f5 - Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Aaron Ballman via cfe-commits]

Author: Ken Matsui
Date: 2022-05-12T09:26:44-04:00
New Revision: a1545f51a9ef299ca6c6716bd80b862f360453ab

URL: 
https://github.com/llvm/llvm-project/commit/a1545f51a9ef299ca6c6716bd80b862f360453ab
DIFF: 
https://github.com/llvm/llvm-project/commit/a1545f51a9ef299ca6c6716bd80b862f360453ab.diff

LOG: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

This adds an extension warning when using the preprocessor conditionals
in a language mode they're not officially supported in, and an opt-in
warning for compatibility with previous standards.

Fixes #55306
Differential Revision: https://reviews.llvm.org/D125178

Added: 
clang/test/Preprocessor/ext-pp-directive.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/lib/Lex/PPDirectives.cpp
clang/test/Lexer/Inputs/unsafe-macro-2.h
clang/test/Lexer/deprecate-macro.c
clang/test/Preprocessor/elifdef.c
clang/test/Preprocessor/if_warning.c
clang/test/Preprocessor/ifdef-recover.c
clang/test/Preprocessor/macro_misc.c
clang/test/Preprocessor/macro_vaopt_check.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6d2a1b8885ce9..e7dd0d1d6e910 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -217,6 +217,12 @@ Improvements to Clang's diagnostics
 - Added the ``-Wgnu-line-marker`` diagnostic flag (grouped under the ``-Wgnu``
   flag) which is a portability warning about use of GNU linemarker preprocessor
   directives. Fixes `Issue 55067 
`_.
+- Using ``#elifdef`` and ``#elifndef`` that are incompatible with C/C++
+  standards before C2x/C++2b are now warned via ``-pedantic``. Additionally,
+  on such language mode, ``-Wpre-c2x-compat`` and ``-Wpre-c++2b-compat``
+  diagnostic flags report a compatibility issue.
+  Fixes `Issue 55306 `_.
+
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 01866d9d0eb7b..3622da94f67e2 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -698,6 +698,23 @@ def warn_cxx98_compat_pp_line_too_big : Warning<
   "#line number greater than 32767 is incompatible with C++98">,
   InGroup, DefaultIgnore;
 
+def warn_c2x_compat_pp_directive : Warning<
+  "use of a '#%select{|elifdef|elifndef}0' directive "
+  "is incompatible with C standards before C2x">,
+  InGroup, DefaultIgnore;
+def ext_c2x_pp_directive : ExtWarn<
+  "use of a '#%select{|elifdef|elifndef}0' directive "
+  "is a C2x extension">,
+  InGroup;
+def warn_cxx2b_compat_pp_directive : Warning<
+  "use of a '#%select{|elifdef|elifndef}0' directive "
+  "is incompatible with C++ standards before C++2b">,
+  InGroup, DefaultIgnore;
+def ext_cxx2b_pp_directive : ExtWarn<
+  "use of a '#%select{|elifdef|elifndef}0' directive "
+  "is a C++2b extension">,
+  InGroup;
+
 def err_pp_visibility_non_macro : Error<"no macro named %0">;
 
 def err_pp_arc_cf_code_audited_syntax : Error<"expected 'begin' or 'end'">;

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 1ef1ba16bcd31..7c7625ad50ba2 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -652,6 +652,17 @@ void 
Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
 PPConditionalInfo &CondInfo = CurPPLexer->peekConditionalLevel();
 Token DirectiveToken = Tok;
 
+// Warn if using `#elifdef` & `#elifndef` in not C2x & C++2b mode even
+// if this branch is in a skipping block.
+unsigned DiagID;
+if (LangOpts.CPlusPlus)
+  DiagID = LangOpts.CPlusPlus2b ? diag::warn_cxx2b_compat_pp_directive
+: diag::ext_cxx2b_pp_directive;
+else
+  DiagID = LangOpts.C2x ? diag::warn_c2x_compat_pp_directive
+: diag::ext_c2x_pp_directive;
+Diag(Tok, DiagID) << (IsElifDef ? PED_Elifdef : PED_Elifndef);
+
 // If this is a #elif with a #else before it, report the error.
 if (CondInfo.FoundElse)
   Diag(Tok, diag::pp_err_elif_after_else)
@@ -3259,6 +3270,23 @@ void Preprocessor::HandleElifFamilyDirective(Token 
&ElifToken,
  : PED_Elifndef;
   ++NumElse;
 
+  // Warn if using `#elifdef` & `#elifndef` in not C2x & C++2b mode.
+  switch (DirKind) {
+  case PED_Elifdef:
+  case PED_Elifndef:
+unsigned DiagID;
+if (LangOpts.CPlusPlus)
+  DiagID = LangOpts.CPlusPlus2b ? diag::warn_cxx2b_compat_pp_directive
+: diag::ext_cxx2b_pp_directive;
+els

[PATCH] D125178: Warn if using `elifdef` & `elifndef` in not C2x & C++2b mode

[Aaron Ballman 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 rGa1545f51a9ef: Warn if using `elifdef` & `elifndef` in 
not C2x & C++2b mode (authored by ken-matsui, committed by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D125178?vs=428910&id=428924#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125178

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/Inputs/unsafe-macro-2.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Preprocessor/elifdef.c
  clang/test/Preprocessor/ext-pp-directive.c
  clang/test/Preprocessor/if_warning.c
  clang/test/Preprocessor/ifdef-recover.c
  clang/test/Preprocessor/macro_misc.c
  clang/test/Preprocessor/macro_vaopt_check.cpp

Index: clang/test/Preprocessor/macro_vaopt_check.cpp
===
--- clang/test/Preprocessor/macro_vaopt_check.cpp
+++ clang/test/Preprocessor/macro_vaopt_check.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++20
-// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -pedantic -std=c++11
-// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -pedantic -std=c99
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++20
+// RUN: %clang_cc1 %s -Eonly -verify -Wno-all -Wno-c++2b-extensions -pedantic -std=c++11
+// RUN: %clang_cc1 -x c %s -Eonly -verify -Wno-all -Wno-c2x-extensions -pedantic -std=c99
 
 //expected-error@+1{{missing '('}}
 #define V1(...) __VA_OPT__  
Index: clang/test/Preprocessor/macro_misc.c
===
--- clang/test/Preprocessor/macro_misc.c
+++ clang/test/Preprocessor/macro_misc.c
@@ -4,6 +4,7 @@
 #ifdef defined
 #elifdef defined
 #endif
+// expected-warning@-2 {{use of a '#elifdef' directive is a C2x extension}}
 
 
 
Index: clang/test/Preprocessor/ifdef-recover.c
===
--- clang/test/Preprocessor/ifdef-recover.c
+++ clang/test/Preprocessor/ifdef-recover.c
@@ -19,12 +19,14 @@
 #if f(2
 #endif
 
-/* expected-error@+2 {{macro name missing}} */
+/* expected-error@+3 {{macro name missing}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef
 #endif
 
-/* expected-error@+2 {{macro name must be an identifier}} */
+/* expected-error@+3 {{macro name must be an identifier}} */
+/* expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}} */
 #ifdef FOO
 #elifdef !
 #endif
Index: clang/test/Preprocessor/if_warning.c
===
--- clang/test/Preprocessor/if_warning.c
+++ clang/test/Preprocessor/if_warning.c
@@ -5,6 +5,7 @@
 #if foo   // expected-error {{'foo' is not defined, evaluates to 0}}
 #endif
 
+// expected-warning@+2 {{use of a '#elifdef' directive is a C2x extension}}
 #ifdef foo
 #elifdef foo
 #endif
@@ -14,6 +15,7 @@
 
 
 // PR3938
+// expected-warning@+3 {{use of a '#elifdef' directive is a C2x extension}}
 #if 0
 #ifdef D
 #elifdef D
Index: clang/test/Preprocessor/ext-pp-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/ext-pp-directive.c
@@ -0,0 +1,59 @@
+// For C
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=pre-c2x-pedantic -pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=pre-c2x-compat -Wpre-c2x-compat %s
+// RUN: not %clang_cc1 -std=c99 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+// For C++
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=pre-cpp2b-pedantic -pedantic %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=pre-cpp2b-compat -Wpre-c++2b-compat %s
+// RUN: not %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify -pedantic %s
+// RUN: not %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify %s
+
+int x;
+
+#if 1
+#elifdef A // #1
+#endif
+// For C
+// pre-c2x-pedantic-warning@#1 {{use of a '#elifdef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warning@#1 {{use of a '#elifdef' directive is a C++2b extension}}
+// pre-cpp2b-compat-warning@#1 {{use of a '#elifdef' directive is incompatible with C++ standards before C++2b}}
+
+#if 1
+#elifndef B // #2
+#endif
+// For C
+// pre-c2x-pedantic-warning@#2 {{use of a '#elifndef' directive is a C2x extension}}
+// pre-c2x-compat-warning@#2 {{use of a '#elifndef' directive is incompatible with C standards before C2x}}
+
+// For C++
+// pre-cpp2b-pedantic-warnin

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:5203
+  T.isConstQualified() &&
+  T->isStructureOrClassType()) {
+const SourceLocation ConstLoc = D.getDeclSpec().getConstSpecLoc();

I wonder if this is same concern applies to Unions?  Should this just be 
`T->isRecordType()`?  Should we do more analysis to ensure that this is a 
movable type? I THINK `CXXRecordDecl::defaultedMoveConstructorIsDeleted()` 
should be enough to test for t his.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125402

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

[PATCH] D124702: [MSVC] Add support for pragma function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Preprocessor/pragma_microsoft.c:210
+#pragma function(main)   // expected-warning {{'main' is not a 
recognized builtin; consider including }}
+#pragma function(// expected-warning {{missing ')' 
after}}
+#pragma function(int)// expected-warning {{missing ')' 
after}}

steplong wrote:
> Hmm does it make sense for this to be a warning and not an error?
I'm glad I'm not the only one who questioned this. :-D I tend to be of the 
opinion that syntax errors should be errors rather than warnings. However, this 
is using an existing warning that's used by a bunch of other pragmas so I think 
it's defensible to use it here, especially given that this pragma form is 
commonly used for compiler extensions (so who knows, the syntax could be valid 
for another compiler, I guess).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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

[clang] 1474244 - Reland "[analyzer] Canonicalize SymIntExpr so the RHS is positive when possible"

[Balazs Benics via cfe-commits]

Author: Tomasz Kamiński
Date: 2022-05-12T15:40:11+02:00
New Revision: 14742443a25826547e480189657b16c7a11664e7

URL: 
https://github.com/llvm/llvm-project/commit/14742443a25826547e480189657b16c7a11664e7
DIFF: 
https://github.com/llvm/llvm-project/commit/14742443a25826547e480189657b16c7a11664e7.diff

LOG: Reland "[analyzer] Canonicalize SymIntExpr so the RHS is positive when 
possible"

This PR changes the `SymIntExpr` so the expression that uses a
negative value as `RHS`, for example: `x +/- (-N)`, is modeled as
`x -/+ N` instead.

This avoids producing a very large `RHS` when the symbol is cased to
an unsigned number, and as consequence makes the value more robust in
presence of casts.

Note that this change is not applied if `N` is the lowest negative
value for which negation would not be representable.

Reviewed By: steakhal

Patch By: tomasz-kaminski-sonarsource!

Differential Revision: https://reviews.llvm.org/D124658

Added: 
clang/test/Analysis/additive-op-on-sym-int-expr.c

Modified: 
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
clang/test/Analysis/expr-inspection.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp 
b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 65c2564637c18..03a751845fe1e 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -104,6 +104,23 @@ SVal SimpleSValBuilder::evalComplement(NonLoc X) {
   }
 }
 
+// Checks if the negation the value and flipping sign preserve
+// the semantics on the operation in the resultType
+static bool isNegationValuePreserving(const llvm::APSInt &Value,
+  APSIntType ResultType) {
+  const unsigned ValueBits = Value.getSignificantBits();
+  if (ValueBits == ResultType.getBitWidth()) {
+// The value is the lowest negative value that is representable
+// in signed integer with bitWith of result type. The
+// negation is representable if resultType is unsigned.
+return ResultType.isUnsigned();
+  }
+
+  // If resultType bitWith is higher that number of bits required
+  // to represent RHS, the sign flip produce same value.
+  return ValueBits < ResultType.getBitWidth();
+}
+
 
//===--===//
 // Transfer function for binary operators.
 
//===--===//
@@ -197,6 +214,17 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
   if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType())
 ConvertedRHS = &BasicVals.Convert(SymbolType, RHS);
 }
+  } else if (BinaryOperator::isAdditiveOp(op) && RHS.isNegative()) {
+// Change a+(-N) into a-N, and a-(-N) into a+N
+// Adjust addition/subtraction of negative value, to
+// subtraction/addition of the negated value.
+APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
+if (isNegationValuePreserving(RHS, resultIntTy)) {
+  ConvertedRHS = &BasicVals.getValue(-resultIntTy.convert(RHS));
+  op = (op == BO_Add) ? BO_Sub : BO_Add;
+} else {
+  ConvertedRHS = &BasicVals.Convert(resultTy, RHS);
+}
   } else
 ConvertedRHS = &BasicVals.Convert(resultTy, RHS);
 
@@ -636,16 +664,26 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
   const llvm::APSInt &first = 
IntType.convert(symIntExpr->getRHS());
   const llvm::APSInt &second = IntType.convert(*RHSValue);
 
+  // If the op and lop agrees, then we just need to
+  // sum the constants. Otherwise, we change to operation
+  // type if substraction would produce negative value
+  // (and cause overflow for unsigned integers),
+  // as consequence x+1U-10 produces x-9U, instead
+  // of x+4294967287U, that would be produced without this
+  // additional check.
   const llvm::APSInt *newRHS;
-  if (lop == op)
+  if (lop == op) {
 newRHS = BasicVals.evalAPSInt(BO_Add, first, second);
-  else
+  } else if (first >= second) {
 newRHS = BasicVals.evalAPSInt(BO_Sub, first, second);
+op = lop;
+  } else {
+newRHS = BasicVals.evalAPSInt(BO_Sub, second, first);
+  }
 
   assert(newRHS && "Invalid operation despite common type!");
   rhs = nonloc::ConcreteInt(*newRHS);
   lhs = nonloc::SymbolVal(symIntExpr->getLHS());
-  op = lop;
   continue;
 }
   }

diff  --git a/clang/test/Analysis/additive-op-on-sym-int-expr.c 
b/clang/test/Analysis/additive-op-on-sym-int-expr.c
new file mode 100644
index 0..bd951c4443a1b
--- /dev/null
+++ b/clang/test/Analysis/additiv

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Balázs Benics 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 rG14742443a258: Reland "[analyzer] Canonicalize 
SymIntExpr so the RHS is positive when possible" (authored by 
tomasz-kaminski-sonarsource, committed by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -11,7 +11,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
-  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) - 1}}
   int y = 1;
   for (; y < 3; ++y) {
 clang_analyzer_numTimesReached(); // expected-warning{{2}}
Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- /dev/null
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -0,0 +1,169 @@
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu -analyzer-checker=core,apiModeling,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_dump(int);
+void clang_analyzer_dumpL(long);
+void clang_analyzer_warnIfReached();
+
+void testInspect(int x) {
+  if ((x < 10) || (x > 100)) {
+return;
+  }
+  // x: [10, 100]
+
+  int i = x + 1;
+  long l = i - 10U;
+  clang_analyzer_dump(i);   // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+  clang_analyzer_dumpL(l);  // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+  clang_analyzer_dumpL(l + 0L); // expected-warning-re {{(reg_${{[0-9]+}}) - 9 }}  instead of + 4294967287
+
+  if ((l - 1000) > 0) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000L) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if ((l + 0L) > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+
+  i = x - 1;
+  l = i + 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) + 9U }} instead of - 4294967287U
+
+  i = x + (-1);
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 11U }} instead of + 4294967285U
+
+  i = x + 1U;
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+}
+
+const int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negation value is not representable
+const long longMin = 1L << (sizeof(long) * 8 - 1); // LONG_MIN, negation value is not representable
+
+void testMin(int i, long l) {
+  clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+  clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+
+  // Do not normalize representation if negation would not be representable
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} no change
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }} no change
+  // Produced value has higher bit with (long) so negation if representable
+  clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
+  clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
+}
+
+void changingToUnsinged(unsigned u, int i) {
+  unsigned c = u + (unsigned)i;
+  unsigned d = u - (unsigned)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+return;
+  }
+}
+
+void extendingToSigned(long l, int i) {
+  long c = l + (long)i;
+  long d = l - (long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483

[PATCH] D124702: [MSVC] Add support for pragma function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:1070
+SourceLocation Loc, const llvm::SmallVectorImpl &NoBuiltins) {
+  if (!CurContext->isFileContext()) {
+Diag(Loc, diag::err_pragma_expected_file_scope) << "function";

It looks like we need `getRedeclContext()` after all, consider:
```
#include 

extern "C" {
void foo();
#pragma function(memset)
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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

[PATCH] D122126: [LoopVectorize] Don't interleave when the number of runtime checks exceeds the threshold

[Tiehu Zhang via Phabricator via cfe-commits]

TiehuZhang updated this revision to Diff 428930.
TiehuZhang added a comment.

(Updated)
Difference with accepted version:  Move memory runtime checks to processLoop to 
control both VF and IC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122126

Files:
  llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  
llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll

Index: llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll
===
--- /dev/null
+++ llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll
@@ -0,0 +1,86 @@
+; RUN: opt -mtriple=powerpc64-unknown-linux-gnu -mcpu=a2 -S -loop-vectorize  < %s -o - | FileCheck %s
+ 
+; The case will do aggressive interleave on PowerPC, resulting in a lot of memory checks.
+; (On the A2, always unroll aggressively. In fact, if aggressive interleaving is enabled,
+; similar issues may occur on other targets).
+; Interleaving should also be restricted by the threshold of memory checks similar to VF.
+; (e.g., runtime-memory-check-threshold, default 8).
+ 
+; CHECK-LABEL: @eddy_diff_caleddy_
+; CHECK-NOT: vector.memcheck
+ 
+define fastcc void @eddy_diff_caleddy_(i64* %wet_cl, i64 %0, i32 %ncol.cast.val) {
+entry:
+  %trip.count = add nuw i32 %ncol.cast.val, 1
+  %wide.trip.count = zext i32 %ncol.cast.val to i64
+  %1 = shl i64 %0, 1
+  %2 = mul i64 %0, 3
+  %3 = shl i64 %0, 2
+  %4 = mul i64 %0, 5
+  %5 = mul i64 %0, 6
+  %6 = mul i64 %0, 7
+  %7 = shl i64 %0, 3
+  %8 = mul i64 %0, 9
+  %9 = mul i64 %0, 10
+  %10 = mul i64 %0, 11
+  %11 = mul i64 %0, 12
+  br label %loop.body
+ 
+loop.body:
+  %indvars.iv774 = phi i64 [ 0, %entry ], [ %indvars.iv.next775, %loop.body ]
+  %12 = add nsw i64 %indvars.iv774, -5
+  %13 = add i64 %12, %0
+  %14 = getelementptr i64, i64* %wet_cl, i64 %13
+  %15 = bitcast i64* %14 to double*
+  store double 0.00e+00, double* %15, align 8
+  %16 = add i64 %12, %1
+  %17 = getelementptr i64, i64* %wet_cl, i64 %16
+  %18 = bitcast i64* %17 to double*
+  store double 0.00e+00, double* %18, align 8
+  %19 = add i64 %12, %2
+  %20 = getelementptr i64, i64* %wet_cl, i64 %19
+  %21 = bitcast i64* %20 to double*
+  store double 0.00e+00, double* %21, align 8
+  %22 = add i64 %12, %3
+  %23 = getelementptr i64, i64* %wet_cl, i64 %22
+  %24 = bitcast i64* %23 to double*
+  store double 0.00e+00, double* %24, align 8
+  %25 = add i64 %12, %4
+  %26 = getelementptr i64, i64* %wet_cl, i64 %25
+  %27 = bitcast i64* %26 to double*
+  store double 0.00e+00, double* %27, align 8
+  %28 = add i64 %12, %5
+  %29 = getelementptr i64, i64* %wet_cl, i64 %28
+  %30 = bitcast i64* %29 to double*
+  store double 0.00e+00, double* %30, align 8
+  %31 = add i64 %12, %6
+  %32 = getelementptr i64, i64* %wet_cl, i64 %31
+  %33 = bitcast i64* %32 to double*
+  store double 0.00e+00, double* %33, align 8
+  %34 = add i64 %12, %7
+  %35 = getelementptr i64, i64* %wet_cl, i64 %34
+  %36 = bitcast i64* %35 to double*
+  store double 0.00e+00, double* %36, align 8
+  %37 = add i64 %12, %8
+  %38 = getelementptr i64, i64* %wet_cl, i64 %37
+  %39 = bitcast i64* %38 to double*
+  store double 0.00e+00, double* %39, align 8
+  %40 = add i64 %12, %9
+  %41 = getelementptr i64, i64* %wet_cl, i64 %40
+  %42 = bitcast i64* %41 to double*
+  store double 0.00e+00, double* %42, align 8
+  %43 = add i64 %12, %10
+  %44 = getelementptr i64, i64* %wet_cl, i64 %43
+  %45 = bitcast i64* %44 to double*
+  store double 0.00e+00, double* %45, align 8
+  %46 = add i64 %12, %11
+  %47 = getelementptr i64, i64* %wet_cl, i64 %46
+  %48 = bitcast i64* %47 to double*
+  store double 0.00e+00, double* %48, align 8
+  %indvars.iv.next775 = add nuw nsw i64 %indvars.iv774, 1
+  %exitcond778.not = icmp eq i64 %indvars.iv.next775, %wide.trip.count
+  br i1 %exitcond778.not, label %loop.end, label %loop.body
+ 
+loop.end:
+  ret void
+}
Index: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
===
--- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7471,6 +7471,14 @@
   return VectorizationFactor::Disabled();
 }
 
+bool LoopVectorizationPlanner::requiresTooManyRuntimeChecks() {
+  unsigned NumRuntimePointerChecks = Requirements.getNumRuntimePointerChecks();
+  return (NumRuntimePointerChecks >
+  VectorizerParams::RuntimeMemoryCheckThreshold &&
+  !Hints.allowReordering()) ||
+ NumRuntimePointerChecks > PragmaVectorizeMemoryCheckThreshold;
+}
+
 Optional
 LoopVectorizationPlanner::plan(ElementCount UserVF, unsigned UserIC) {
   assert(OrigLoop->isInnermost() && "Inner loop expected.");
@@ -7538,31 +7546,7 @@

[PATCH] D122126: [LoopVectorize] Don't interleave when the number of runtime checks exceeds the threshold

[Tiehu Zhang via Phabricator via cfe-commits]

TiehuZhang added a comment.

The code has been updated since accept. Please review it again. Thank you very 
much! @fhahn @dmgreen




Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10460
 IC = CM.selectInterleaveCount(VF.Width, *VF.Cost.getValue());
+if (!UserIC && requiresTooManyRtChecks) {
+  ORE->emit([&]() {

fhahn wrote:
> Can the handling be merged into a single check & diagnostic?
Hi，@fhahn, thanks for your reply! Does the current version meet the 
requirements？



Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10460
 IC = CM.selectInterleaveCount(VF.Width, *VF.Cost.getValue());
+if (!UserIC && requiresTooManyRtChecks) {
+  ORE->emit([&]() {

TiehuZhang wrote:
> fhahn wrote:
> > Can the handling be merged into a single check & diagnostic?
> Hi，@fhahn, thanks for your reply! Does the current version meet the 
> requirements？
Hi, @fhahn, is there any other problem with this patch? 

ping



Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10461
+if (!LVP.hasTooManyRuntimeChecks()) {
+  IC = CM.selectInterleaveCount(VF.Width, *VF.Cost.getValue());
+}

fhahn wrote:
> This check here should be sufficient, there should be no need to also check 
> in `selectInterleaveCount`.
> 
> Could you just move the remark generation & early exit from `::plan` here?
> 
> You might want to skip those checks if there's a UserVF or UserIC used, with 
> those I think we should always vectorize if possible. It also might be good 
> to add a check line to your test which forces an interleave count > 1.
Hi, @fhahn, thanks for your review! It sounds similar to doesNotMeet 
(https://reviews.llvm.org/D98634), but the difference is that I need to use 
UserIC and UserVF to control whether this check needs to be performed, right? 
E.g.

```
if (!UserVF && LVP.requiresTooManyRuntimeChecks()) {
  /*generate remarks*/
  VF = VectorizationFactor::Disabled();
}

if (!UserIC && LVP.requiresTooManyRuntimeChecks()) {
  /*generate remarks*/
  IC = 1;
}
```

> Could you just move the remark generation & early exit from ::plan here?
> 
> You might want to skip those checks if there's a UserVF or UserIC used, with 
> those I think we should always vectorize if possible. It also might be good 
> to add a check line to your test which forces an interleave count > 1.







Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7673-7674
   // Check if it is profitable to vectorize with runtime checks.
-  unsigned NumRuntimePointerChecks = Requirements.getNumRuntimePointerChecks();
-  if (SelectedVF.Width.getKnownMinValue() > 1 && NumRuntimePointerChecks) {
-bool PragmaThresholdReached =
-NumRuntimePointerChecks > PragmaVectorizeMemoryCheckThreshold;
-bool ThresholdReached =
-NumRuntimePointerChecks > 
VectorizerParams::RuntimeMemoryCheckThreshold;
-if ((ThresholdReached && !Hints.allowReordering()) ||
-PragmaThresholdReached) {
+  if (SelectedVF.Width.getKnownMinValue() > 1) {
+if (hasTooManyRuntimeChecks()) {
   ORE->emit([&]() {

dmgreen wrote:
> Maybe just use a single if now: `if (SelectedVF.Width.getKnownMinValue() > 1 
> && hasTooManyRuntimeChecks()) {`
Done. Thanks for your review! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122126

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

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

[Gabor Marton via Phabricator via cfe-commits]

martong marked 13 inline comments as done.
martong added a comment.

In D125395#3506854 , @steakhal wrote:

> Great content!
> I've got a long list of nits though. Nothing personal :D

No worries, thank you for being assiduous.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1454-1456
+  if (USE->getOpcode() == UO_Minus) {
+// Just get the operand when we negate a symbol that is already 
negated.
+// -(-a) == a, ~(~a) == a

steakhal wrote:
> The comment says that it would work with both `-` and `~`.
> Why do you check against only `-`?
Unfortunately, the complement operation is not implemented on the RangeSet. So, 
yes, in this sense the comment is misleading, I've changed it.



Comment at: clang/test/Analysis/constraint_manager_negate.c:27-29
+  clang_analyzer_eval(a < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= INT_MIN); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == INT_MIN); // expected-warning{{FALSE}}

steakhal wrote:
> You can also query if the bound is sharp, by asking the same question but 
> with a slightly different value and expect `UNKNOWN`.
> ```
>   clang_analyzer_eval(a < 0); // expected-warning{{TRUE}}
>   clang_analyzer_eval(a < -1); // expected-warning{{UNKNOWN}}
> ```
> 
> I also believe that `a >= INT_MIN` is `TRUE` for all `a` anyway, thus it can 
> be omitted.
> Checking against equality won't help much either, because we had no equality 
> check before, thus it should result in `FALSE` unconditionally.
Okay, I changed it to something very obvious:
```
  clang_analyzer_eval(a < 0); // expected-warning{{TRUE}}
  clang_analyzer_eval(a > INT_MIN); // expected-warning{{TRUE}}
```



Comment at: clang/test/Analysis/constraint_manager_negate.c:46
+return;
+  clang_analyzer_eval(-a == INT_MIN); // expected-warning{{TRUE}}
+}

steakhal wrote:
> Let's also assert `a == INT_MIN` just for the symmetry.
That would be superfluous in my opinion, since I have changed the `if` to an
```
  assert(a == INT_MIN);
```



Comment at: clang/test/Analysis/constraint_manager_negate.c:76
+void negate_unsigned_min(unsigned a) {
+  if (a == UINT_MIN) {
+clang_analyzer_eval(-a == UINT_MIN); // expected-warning{{TRUE}}

steakhal wrote:
> Well, this is the third form of expressing constraints.
> 1) Using asserts.
> 2) Use ifs but with early returns.
> 3) Use ifs but embed the code into the true branch.
> 
> Please, settle on one method.
Ok, I changed to `assert` everywhere.



Comment at: clang/test/Analysis/constraint_manager_negate.c:87-88
+  if (a == 4u) {
+clang_analyzer_eval(-a == 4u); // expected-warning{{FALSE}}
+clang_analyzer_eval(-a != 4u); // expected-warning{{TRUE}}
+  }

steakhal wrote:
> Add an equality comparison which results in `TRUE`.
Sure.



Comment at: clang/test/Analysis/constraint_manager_negate.c:100-105
+void negate_unsigned_mid2(unsigned a) {
+  if (a < UINT_MID && a > UINT_MIN) {
+clang_analyzer_eval(-a > UINT_MID); // expected-warning{{TRUE}}
+clang_analyzer_eval(-a < UINT_MID); // expected-warning{{FALSE}}
+  }
+}

steakhal wrote:
> I believe the eval query can be sharper than this.
Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125395

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

[PATCH] D125395: [analyzer][solver] Handle UnarySymExpr in RangeConstraintSolver

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 428932.
martong marked 6 inline comments as done.
martong added a comment.

- Address steakhal review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125395

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constraint_manager_negate.c
  clang/test/Analysis/constraint_manager_negate_difference.c
  clang/test/Analysis/unary-sym-expr.c

Index: clang/test/Analysis/unary-sym-expr.c
===
--- clang/test/Analysis/unary-sym-expr.c
+++ clang/test/Analysis/unary-sym-expr.c
@@ -28,3 +28,12 @@
 return;
   clang_analyzer_eval(-(x + y) == -3); // expected-warning{{TRUE}}
 }
+
+int test_fp(int flag) {
+  int value;
+  if (flag == 0)
+value = 1;
+  if (-flag == 0)
+return value; // no-warning
+  return 42;
+}
Index: clang/test/Analysis/constraint_manager_negate_difference.c
===
--- clang/test/Analysis/constraint_manager_negate_difference.c
+++ clang/test/Analysis/constraint_manager_negate_difference.c
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-binary-operation-simplification=true -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify %s
 
 void clang_analyzer_eval(int);
 
@@ -10,11 +12,8 @@
 #define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
 #define INT_MIN (UINT_MAX & ~(UINT_MAX >> 1))
 
-extern void __assert_fail (__const char *__assertion, __const char *__file,
-unsigned int __line, __const char *__function)
- __attribute__ ((__noreturn__));
-#define assert(expr) \
-  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+extern void abort() __attribute__((__noreturn__));
+#define assert(expr) ((expr) ? (void)(0) : abort())
 
 void assert_in_range(int x) {
   assert(x <= ((int)INT_MAX / 4));
@@ -33,69 +32,60 @@
 
 void equal(int m, int n) {
   assert_in_range_2(m, n);
-  if (m != n)
-return;
+  assert(m == n);
   assert_in_wide_range(m - n);
   clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
 }
 
 void non_equal(int m, int n) {
   assert_in_range_2(m, n);
-  if (m == n)
-return;
+  assert(m != n);
   assert_in_wide_range(m - n);
   clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
 }
 
 void less_or_equal(int m, int n) {
   assert_in_range_2(m, n);
-  if (m < n)
-return;
+  assert(m >= n);
   assert_in_wide_range(m - n);
   clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
 }
 
 void less(int m, int n) {
   assert_in_range_2(m, n);
-  if (m <= n)
-return;
+  assert(m > n);
   assert_in_wide_range(m - n);
   clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
 }
 
 void greater_or_equal(int m, int n) {
   assert_in_range_2(m, n);
-  if (m > n)
-return;
+  assert(m <= n);
   assert_in_wide_range(m - n);
   clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
 }
 
 void greater(int m, int n) {
   assert_in_range_2(m, n);
-  if (m >= n)
-return;
+  assert(m < n);
   assert_in_wide_range(m - n);
   clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
 }
 
 void negate_positive_range(int m, int n) {
-  if (m - n <= 0)
-return;
+  assert(m - n > 0);
   clang_analyzer_eval(n - m < 0); // expected-warning{{TRUE}}
   clang_analyzer_eval(n - m > INT_MIN); // expected-warning{{TRUE}}
-  clang_analyzer_eval(n - m == INT_MIN); // expected-warning{{FALSE}}
 }
 
+_Static_assert(INT_MIN == -INT_MIN, "");
 void negate_int_min(int m, int n) {
-  if (m - n != INT_MIN)
-return;
+  assert(m - n == INT_MIN);
   clang_analyzer_eval(n - m == INT_MIN); // expected-warning{{TRUE}}
 }
 
 void negate_mixed(int m, int n) {
-  if (m -n > INT_MIN && m - n <= 0)
-return;
+  assert(m - n > 0 || m - n == INT_MIN);
   clang_analyzer_eval(n - m <= 0); // expected-warning{{TRUE}}
 }
 
@@ -106,54 +96,59 @@
   clang_analyzer_eval(n - m == 0); // expected-warning{{TRUE}}
 }
 
+_Static_assert(INT_MIN == -INT_MIN, "");
 void effective_range_2(int m, int n) {
   assert(m - n <= 0);
   assert(n - m <= 0);
-  clang_analyzer_eval(m - n == 0); // expected-warning{{TRUE}} expected-warning{{FALSE}}
-  clang_analyzer_eval(n - m == 0); // expected-warning{{TRUE}} expected-warning{{FALSE}}
+  clang_analyzer_eval(m - n == 0 || m - n == INT_MIN); // expected-warning{{TRUE}}
 }
 
 void negate_unsigned_min(unsigned m, unsigned n) {
-  if (m - n == UINT_MIN) {
-clang_analyzer_eval(n - m == UINT_MIN); // expected-warning{{TRUE}}
-clang_analyzer_eval(n - m != UINT_MIN); // expected-warning{{FALSE}}
-clang_analyzer_eval(n - m > UINT_MIN);  // expected-warning{{FALSE}}
-clang_analyzer_eval(n - m < UINT_MIN);  // expected-warning{{FALSE}}
-  }
+  assert(m - n == UINT_MIN);
+  clang

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource added a comment.

Regarding the "why":
Our tools (SonarQube / SonarCloud / SonarLint) use the clang front-end to parse 
our customer's code and find bugs/bad patterns in it. Said customer code can 
target various compilers including MSVC and we need to handle it as gracefully 
as possible.
When we find gaps between MSVC and clang-cl, we try to fix them and if we think 
the fix will have negligible memory/performance/complexity impact on clang and 
be useful to the community, we try to upstream them. It's true that this fix 
purpose is not to fix handling of MSVC standard headers, but there are more and 
more tools that also target existing MSVC code (clang-tidy, Clang Power Tools, 
etc.) thanks to this compatibility feature and we felt that it could be 
valuable.
We also try to always have a warning attached to MSVC extensions, but this one 
has two particularities: it builds on top of an already existing MSVC 
compatibility feature (PerformFriendInjection in Decl::setObjectOfFriendDecl) 
and there is no way (without a big refactoring), that would allow to warn 
correctly when the extension is actually used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124613

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

[PATCH] D125011: [MSVC] Add support for pragma alloc_text

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor commenting nits that you can fix when landing. Can 
you also add a release note for the new pragma support?




Comment at: clang/include/clang/Sema/Sema.h:727
 
+  /// Sections used with #pragma alloc_text
+  llvm::StringMap> FunctionToSectionMap;

aaron.ballman wrote:
> 
Looks like this one was missed.



Comment at: clang/include/clang/Sema/Sema.h:10337
 
+  void AddSectionMSAllocText(FunctionDecl *FD);
+

aaron.ballman wrote:
> This could probably have some comments with it.
Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125011

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

[PATCH] D125463: [analyzer][NFC] Tighten some of the SValBuilder return types

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Seems like it compiles, so LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125463

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

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Function `mkdir` is modeled incorrectly by the checker. According to the man 
page it can return 0 or -1 only (-1 is error) but the checker allows 
non-negative value at success. So the shown bug report is incorrect (it can be 
only -1 if not 0 and then check of `errno` is allowed). Anyway the note tags 
should be added to every function.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1697
+.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked, "OK")
+.Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant, "'dup2' failed")
 .ArgConstraint(ArgumentCondition(0, WithinRange, Range(0, IntMax)))

These strings are for test purposes only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125400

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

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do you expect to use this matcher in a new check in the immediate future? We 
usually don't push specialized matchers into ASTMatchers.h but instead try to 
keep them next to the only check using them. This reduces compile time overhead 
for everyone building Clang. (It's not that there's anything wrong with this 
one, it's more just resistance to adding matches unless they're sufficiently 
necessary.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125383

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Tom Honermann via Phabricator via cfe-commits]

tahonermann accepted this revision as: tahonermann.
tahonermann added a comment.
This revision is now accepted and ready to land.

Looks good, @tbaeder! Thank you for sticking with me through all these 
iterations!


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

https://reviews.llvm.org/D124996

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

[clang] 646e502 - [clang] add -fmodule-file-home-is-cwd

[Richard Howell via cfe-commits]

Author: Richard Howell
Date: 2022-05-12T07:27:47-07:00
New Revision: 646e502de0d854cb3ecaca90ab52bebfe59a40cd

URL: 
https://github.com/llvm/llvm-project/commit/646e502de0d854cb3ecaca90ab52bebfe59a40cd
DIFF: 
https://github.com/llvm/llvm-project/commit/646e502de0d854cb3ecaca90ab52bebfe59a40cd.diff

LOG: [clang] add -fmodule-file-home-is-cwd

This diff adds a new frontend flag `-fmodule-file-home-is-cwd`.
The behavior of this flag is similar to
`-fmodule-map-file-home-is-cwd` but does not require the module
map files to be modified to have inputs relative to the cwd.
Instead the output modules will have their `BaseDirectory` set
to the cwd and will try and resolve paths relative to that.

The motiviation for this change is to support relocatable pcm
files that are built on different machines with different paths
without having to alter module map files, which is sometimes not
possible as they are provided by 3rd parties.

Reviewed By: urnathan

Differential Revision: https://reviews.llvm.org/D124874

Added: 
clang/test/Modules/module-file-home-is-cwd.m

Modified: 
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/HeaderSearchOptions.h
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8e840ede926a9..6668ac4ca9470 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5619,6 +5619,10 @@ def fmodule_map_file_home_is_cwd : Flag<["-"], 
"fmodule-map-file-home-is-cwd">,
   HelpText<"Use the current working directory as the home directory of "
"module maps specified by -fmodule-map-file=">,
   MarshallingInfoFlag>;
+def fmodule_file_home_is_cwd : Flag<["-"], "fmodule-file-home-is-cwd">,
+  HelpText<"Use the current working directory as the base directory of "
+   "compiled module files.">,
+  MarshallingInfoFlag>;
 def fmodule_feature : Separate<["-"], "fmodule-feature">,
   MetaVarName<"">,
   HelpText<"Enable  in module map requires declarations">,

diff  --git a/clang/include/clang/Lex/HeaderSearchOptions.h 
b/clang/include/clang/Lex/HeaderSearchOptions.h
index 4efdfc26c3c67..6436a9b3bde20 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -143,6 +143,12 @@ class HeaderSearchOptions {
   /// file.
   unsigned ModuleMapFileHomeIsCwd : 1;
 
+  /// Set the base path of a built module file to be the current working
+  /// directory. This is useful for sharing module files across machines
+  /// that build with 
diff erent paths without having to rewrite all
+  /// modulemap files to have working directory relative paths.
+  unsigned ModuleFileHomeIsCwd : 1;
+
   /// Also search for prebuilt implicit modules in the prebuilt module cache
   /// path.
   unsigned EnablePrebuiltImplicitModules : 1;
@@ -222,9 +228,9 @@ class HeaderSearchOptions {
   HeaderSearchOptions(StringRef _Sysroot = "/")
   : Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
 ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
-EnablePrebuiltImplicitModules(false), UseBuiltinIncludes(true),
-UseStandardSystemIncludes(true), UseStandardCXXIncludes(true),
-UseLibcxx(false), Verbose(false),
+ModuleFileHomeIsCwd(false), EnablePrebuiltImplicitModules(false),
+UseBuiltinIncludes(true), UseStandardSystemIncludes(true),
+UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
 ModulesValidateOncePerBuildSession(false),
 ModulesValidateSystemHeaders(false),
 ValidateASTInputFilesContent(false), UseDebugInfo(false),

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 228bd9aa08db4..e42f41f8fbc2d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1224,15 +1224,24 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, 
ASTContext &Context,
   }
 
   if (WritingModule && WritingModule->Directory) {
-SmallString<128> BaseDir(WritingModule->Directory->getName());
+SmallString<128> BaseDir;
+if (PP.getHeaderSearchInfo().getHeaderSearchOpts().ModuleFileHomeIsCwd) {
+  // Use the current working directory as the base path for all inputs.
+  auto *CWD =
+  Context.getSourceManager().getFileManager().getDirectory(".").get();
+  BaseDir.assign(CWD->getName());
+} else {
+  BaseDir.assign(WritingModule->Directory->getName());
+}
 cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
 
 // If the home of the module is the current working directory, then we
 // want to pick up the cwd of the build process loading the module, not
 // our cwd, when we load this module.
-if (!PP.getHeaderSearchInfo()
- .getHeaderSearchOpts()
-   

[PATCH] D124874: [clang] add -fmodule-file-home-is-cwd

[Richard Howell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG646e502de0d8: [clang] add -fmodule-file-home-is-cwd 
(authored by rmaz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124874

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/module-file-home-is-cwd.m


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- /dev/null
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -0,0 +1,8 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd 
-fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o 
%t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}a1.h'
+// CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}a2.h'
+// CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}module.map'
+// CHECK-NOT: MODULE_DIRECTORY
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1224,15 +1224,24 @@
   }
 
   if (WritingModule && WritingModule->Directory) {
-SmallString<128> BaseDir(WritingModule->Directory->getName());
+SmallString<128> BaseDir;
+if (PP.getHeaderSearchInfo().getHeaderSearchOpts().ModuleFileHomeIsCwd) {
+  // Use the current working directory as the base path for all inputs.
+  auto *CWD =
+  Context.getSourceManager().getFileManager().getDirectory(".").get();
+  BaseDir.assign(CWD->getName());
+} else {
+  BaseDir.assign(WritingModule->Directory->getName());
+}
 cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
 
 // If the home of the module is the current working directory, then we
 // want to pick up the cwd of the build process loading the module, not
 // our cwd, when we load this module.
-if (!PP.getHeaderSearchInfo()
- .getHeaderSearchOpts()
- .ModuleMapFileHomeIsCwd ||
+if (!(PP.getHeaderSearchInfo()
+  .getHeaderSearchOpts()
+  .ModuleMapFileHomeIsCwd ||
+  PP.getHeaderSearchInfo().getHeaderSearchOpts().ModuleFileHomeIsCwd) 
||
 WritingModule->Directory->getName() != StringRef(".")) {
   // Module directory.
   auto Abbrev = std::make_shared();
Index: clang/include/clang/Lex/HeaderSearchOptions.h
===
--- clang/include/clang/Lex/HeaderSearchOptions.h
+++ clang/include/clang/Lex/HeaderSearchOptions.h
@@ -143,6 +143,12 @@
   /// file.
   unsigned ModuleMapFileHomeIsCwd : 1;
 
+  /// Set the base path of a built module file to be the current working
+  /// directory. This is useful for sharing module files across machines
+  /// that build with different paths without having to rewrite all
+  /// modulemap files to have working directory relative paths.
+  unsigned ModuleFileHomeIsCwd : 1;
+
   /// Also search for prebuilt implicit modules in the prebuilt module cache
   /// path.
   unsigned EnablePrebuiltImplicitModules : 1;
@@ -222,9 +228,9 @@
   HeaderSearchOptions(StringRef _Sysroot = "/")
   : Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
 ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
-EnablePrebuiltImplicitModules(false), UseBuiltinIncludes(true),
-UseStandardSystemIncludes(true), UseStandardCXXIncludes(true),
-UseLibcxx(false), Verbose(false),
+ModuleFileHomeIsCwd(false), EnablePrebuiltImplicitModules(false),
+UseBuiltinIncludes(true), UseStandardSystemIncludes(true),
+UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
 ModulesValidateOncePerBuildSession(false),
 ModulesValidateSystemHeaders(false),
 ValidateASTInputFilesContent(false), UseDebugInfo(false),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5619,6 +5619,10 @@
   HelpText<"Use the current working directory as the home directory of "
"module maps specified by -fmodule-map-file=">,
   MarshallingInfoFlag>;
+def fmodule_file_home_is_cwd : Flag<["-"], "fmodule-file-home-is-cwd">,
+  HelpText<"Use the current working directory as the base directory of "
+   "compiled module files.">,
+  MarshallingInfoFlag>;
 def fmodule_feature : Separate<["-"], "fmodule-feature">,
   MetaVarName<"">,
   HelpText<"Enable  in module map requires declarations">,


Index: clang/test/Modules/module-file-home-is-cwd.m

[clang] f110569 - [clang] serialize SUBMODULE_TOPHEADER relative to BaseDirectory

[Richard Howell via cfe-commits]

Author: Richard Howell
Date: 2022-05-12T07:29:37-07:00
New Revision: f11056943e56a32d81bb36d11fb5ce8d2b2ce79b

URL: 
https://github.com/llvm/llvm-project/commit/f11056943e56a32d81bb36d11fb5ce8d2b2ce79b
DIFF: 
https://github.com/llvm/llvm-project/commit/f11056943e56a32d81bb36d11fb5ce8d2b2ce79b.diff

LOG: [clang] serialize SUBMODULE_TOPHEADER relative to BaseDirectory

This diff changes the serialization of the `SUBMODULE_TOPHEADER`
entry in module files to be serialized relative to the module's
`BaseDirectory`. This matches the behavior of the
`SUBMODULE_HEADER` entry and will allow for the module to be
relocatable across machines.

The path is restored relative to the module's `BaseDirectory` on
deserialization.

Reviewed By: urnathan

Differential Revision: https://reviews.llvm.org/D124938

Added: 
clang/test/Modules/relative-submodule-topheader.m

Modified: 
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index c9601f0a164c9..93ecf222a5b30 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5637,9 +5637,12 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
   // them here.
   break;
 
-case SUBMODULE_TOPHEADER:
-  CurrentModule->addTopHeaderFilename(Blob);
+case SUBMODULE_TOPHEADER: {
+  std::string HeaderName(Blob);
+  ResolveImportedPath(F, HeaderName);
+  CurrentModule->addTopHeaderFilename(HeaderName);
   break;
+}
 
 case SUBMODULE_UMBRELLA_DIR: {
   // See comments in SUBMODULE_UMBRELLA_HEADER

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index e42f41f8fbc2d..4eeedc07fb1d4 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2857,8 +2857,11 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
 {
   auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
   RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
-  for (auto *H : TopHeaders)
-Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, H->getName());
+  for (auto *H : TopHeaders) {
+SmallString<128> HeaderName(H->getName());
+PreparePathForOutput(HeaderName);
+Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
+  }
 }
 
 // Emit the imports.

diff  --git a/clang/test/Modules/relative-submodule-topheader.m 
b/clang/test/Modules/relative-submodule-topheader.m
new file mode 100644
index 0..c7c2f13076866
--- /dev/null
+++ b/clang/test/Modules/relative-submodule-topheader.m
@@ -0,0 +1,10 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x objective-c++ 
-fmodule-name=std -emit-module Inputs/submodules/module.map -o %t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'hash_map.h'
+// CHECK:  blob data = 'hash_map.h'



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

[PATCH] D124938: [clang] serialize SUBMODULE_TOPHEADER relative to BaseDirectory

[Richard Howell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf11056943e56: [clang] serialize SUBMODULE_TOPHEADER relative 
to BaseDirectory (authored by rmaz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124938

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/relative-submodule-topheader.m


Index: clang/test/Modules/relative-submodule-topheader.m
===
--- /dev/null
+++ clang/test/Modules/relative-submodule-topheader.m
@@ -0,0 +1,10 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x objective-c++ 
-fmodule-name=std -emit-module Inputs/submodules/module.map -o %t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'hash_map.h'
+// CHECK:  blob data = 'hash_map.h'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2857,8 +2857,11 @@
 {
   auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
   RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
-  for (auto *H : TopHeaders)
-Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, H->getName());
+  for (auto *H : TopHeaders) {
+SmallString<128> HeaderName(H->getName());
+PreparePathForOutput(HeaderName);
+Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
+  }
 }
 
 // Emit the imports.
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5637,9 +5637,12 @@
   // them here.
   break;
 
-case SUBMODULE_TOPHEADER:
-  CurrentModule->addTopHeaderFilename(Blob);
+case SUBMODULE_TOPHEADER: {
+  std::string HeaderName(Blob);
+  ResolveImportedPath(F, HeaderName);
+  CurrentModule->addTopHeaderFilename(HeaderName);
   break;
+}
 
 case SUBMODULE_UMBRELLA_DIR: {
   // See comments in SUBMODULE_UMBRELLA_HEADER


Index: clang/test/Modules/relative-submodule-topheader.m
===
--- /dev/null
+++ clang/test/Modules/relative-submodule-topheader.m
@@ -0,0 +1,10 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x objective-c++ -fmodule-name=std -emit-module Inputs/submodules/module.map -o %t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'hash_map.h'
+// CHECK:  blob data = 'hash_map.h'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2857,8 +2857,11 @@
 {
   auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
   RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
-  for (auto *H : TopHeaders)
-Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, H->getName());
+  for (auto *H : TopHeaders) {
+SmallString<128> HeaderName(H->getName());
+PreparePathForOutput(HeaderName);
+Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
+  }
 }
 
 // Emit the imports.
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5637,9 +5637,12 @@
   // them here.
   break;
 
-case SUBMODULE_TOPHEADER:
-  CurrentModule->addTopHeaderFilename(Blob);
+case SUBMODULE_TOPHEADER: {
+  std::string HeaderName(Blob);
+  ResolveImportedPath(F, HeaderName);
+  CurrentModule->addTopHeaderFilename(HeaderName);
   break;
+}
 
 case SUBMODULE_UMBRELLA_DIR: {
   // See comments in SUBMODULE_UMBRELLA_HEADER
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ee51e97 - [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

[Richard Howell via cfe-commits]

Author: Richard Howell
Date: 2022-05-12T07:31:19-07:00
New Revision: ee51e9795a31e1280e30179215c27e09927230e2

URL: 
https://github.com/llvm/llvm-project/commit/ee51e9795a31e1280e30179215c27e09927230e2
DIFF: 
https://github.com/llvm/llvm-project/commit/ee51e9795a31e1280e30179215c27e09927230e2.diff

LOG: [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

This diff changes the serialization of the `ORIGINAL_PCH_DIR`
entry in module files to be serialized relative to the module's
`BaseDirectory`. This will allow for the module to be relocatable
across machines.

The path is restored relative to the module's BaseDirectory on
deserialization.

Reviewed By: urnathan

Differential Revision: https://reviews.llvm.org/D124946

Added: 
clang/test/Modules/relative-original-dir.m

Modified: 
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 93ecf222a5b3..85e5b3ecfdd7 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2898,6 +2898,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
 
 case ORIGINAL_PCH_DIR:
   F.OriginalDir = std::string(Blob);
+  ResolveImportedPath(F, F.OriginalDir);
   break;
 
 case MODULE_NAME:

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 4eeedc07fb1d..6d9273ed9a53 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1470,8 +1470,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, 
ASTContext &Context,
 unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
 
 SmallString<128> OutputPath(OutputFile);
-
-SM.getFileManager().makeAbsolutePath(OutputPath);
+PreparePathForOutput(OutputPath);
 StringRef origDir = llvm::sys::path::parent_path(OutputPath);
 
 RecordData::value_type Record[] = {ORIGINAL_PCH_DIR};

diff  --git a/clang/test/Modules/relative-original-dir.m 
b/clang/test/Modules/relative-original-dir.m
new file mode 100644
index ..3ed49142bc30
--- /dev/null
+++ b/clang/test/Modules/relative-original-dir.m
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t/normal-module-map
+// RUN: mkdir -p %t
+// RUN: cp -r %S/Inputs/normal-module-map %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-name=libA 
-emit-module %t/normal-module-map/module.map -o 
%t/normal-module-map/outdir/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram 
%t/normal-module-map/outdir/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'outdir'



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

[PATCH] D124946: [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

[Richard Howell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGee51e9795a31: [clang] serialize ORIGINAL_PCH_DIR relative to 
BaseDirectory (authored by rmaz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124946

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/relative-original-dir.m


Index: clang/test/Modules/relative-original-dir.m
===
--- /dev/null
+++ clang/test/Modules/relative-original-dir.m
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t/normal-module-map
+// RUN: mkdir -p %t
+// RUN: cp -r %S/Inputs/normal-module-map %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-name=libA 
-emit-module %t/normal-module-map/module.map -o 
%t/normal-module-map/outdir/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram 
%t/normal-module-map/outdir/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'outdir'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1470,8 +1470,7 @@
 unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
 
 SmallString<128> OutputPath(OutputFile);
-
-SM.getFileManager().makeAbsolutePath(OutputPath);
+PreparePathForOutput(OutputPath);
 StringRef origDir = llvm::sys::path::parent_path(OutputPath);
 
 RecordData::value_type Record[] = {ORIGINAL_PCH_DIR};
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2898,6 +2898,7 @@
 
 case ORIGINAL_PCH_DIR:
   F.OriginalDir = std::string(Blob);
+  ResolveImportedPath(F, F.OriginalDir);
   break;
 
 case MODULE_NAME:


Index: clang/test/Modules/relative-original-dir.m
===
--- /dev/null
+++ clang/test/Modules/relative-original-dir.m
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t/normal-module-map
+// RUN: mkdir -p %t
+// RUN: cp -r %S/Inputs/normal-module-map %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-name=libA -emit-module %t/normal-module-map/module.map -o %t/normal-module-map/outdir/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/normal-module-map/outdir/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'outdir'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1470,8 +1470,7 @@
 unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
 
 SmallString<128> OutputPath(OutputFile);
-
-SM.getFileManager().makeAbsolutePath(OutputPath);
+PreparePathForOutput(OutputPath);
 StringRef origDir = llvm::sys::path::parent_path(OutputPath);
 
 RecordData::value_type Record[] = {ORIGINAL_PCH_DIR};
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2898,6 +2898,7 @@
 
 case ORIGINAL_PCH_DIR:
   F.OriginalDir = std::string(Blob);
+  ResolveImportedPath(F, F.OriginalDir);
   break;
 
 case MODULE_NAME:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124702: [MSVC] Add support for pragma function

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 428941.
steplong added a comment.

- Added test case for the `getRedeclContext()` case
- Brought back `getRedeclContext()`
- Clang-formatted
- Added description in docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/pragma-ms-function.c
  clang/test/Preprocessor/pragma_microsoft.c
  clang/test/Preprocessor/pragma_microsoft.cpp

Index: clang/test/Preprocessor/pragma_microsoft.cpp
===
--- clang/test/Preprocessor/pragma_microsoft.cpp
+++ clang/test/Preprocessor/pragma_microsoft.cpp
@@ -1,3 +1,7 @@
 // RUN: %clang_cc1 %s -fsyntax-only -std=c++11 -verify -fms-extensions
 
 #pragma warning(push, 4_D) // expected-warning {{requires a level between 0 and 4}}
+
+extern "C" {
+#pragma function(memset) // no-warning
+}
Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -201,6 +201,27 @@
 #pragma clang diagnostic pop
 #pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
 
+// Test pragma function
+#pragma function(memset) // no-warning
+#pragma function(memcpy, strlen, strlen) // no-warning
+#pragma function()   // no-warning
+#pragma function(asdf)   // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+#pragma function(main)   // expected-warning {{'main' is not a recognized builtin; consider including }}
+#pragma function(// expected-warning {{missing ')' after}}
+#pragma function(int)// expected-warning {{missing ')' after}}
+#pragma function(strcmp) asdf// expected-warning {{extra tokens at end}}
+
+#define __INTRIN_H   // there should be no notes after defining __INTRIN_H
+#pragma function(asdf)   // expected-warning-re {{'asdf' is not a recognized builtin{{$
+#pragma function(memset) // no-warning
+#undef __INTRIN_H
+#pragma function(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+
+// MSVC accepts this, but we decide to reject it based on the MS docs saying the pragma must appear at the global level
+void pragma_function_foo() {
+#pragma function(memset) // expected-error {{'#pragma function' can only appear at file scope}}
+}
+
 #pragma optimize  // expected-warning{{missing '(' after '#pragma optimize'}}
 #pragma optimize( // expected-warning{{expected string literal in '#pragma optimize'}}
 #pragma optimize(a// expected-warning{{expected string literal in '#pragma optimize'}}
Index: clang/test/CodeGen/pragma-ms-function.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-ms-function.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+typedef typeof(sizeof(0)) size_t;
+
+void bar(char *s);
+void *memset(void *s, int c, size_t n);
+void *memcpy(void *d, const void *s, size_t n);
+
+// CHECK: define{{.*}} void @foo1({{.*}}) #[[NO_NOBUILTIN:[0-9]+]]
+void foo1(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 0, n);
+  memcpy(d, s, n);
+}
+
+#pragma function(strlen, memset)
+
+// CHECK: define{{.*}} void @foo2({{.*}}) #[[NOBUILTIN_MEMSET:[0-9]+]]
+void foo2(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 1, n);
+  memcpy(d, s, n);
+}
+
+#pragma function(memcpy)
+
+// CHECK: define{{.*}} void @foo3({{.*}}) #[[NOBUILTIN_MEMSET_MEMCPY:[0-9]+]]
+void foo3(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 2, n);
+  memcpy(d, s, n);
+}
+
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET_MEMCPY]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10218,10 +10218,12 @@
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
-  // If this is a function definition, check if we have to apply optnone due to
-  // a pragma.
-  if(D.isFunctionDefinition())
+  // If thi

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D125383#3508960 , @aaron.ballman 
wrote:

> Do you expect to use this matcher in a new check in the immediate future?

D124446  would like to use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125383

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

[PATCH] D125468: [clangd] Include Cleaner: ignore headers with IWYU export pragmas

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: sammccall, kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Disable the warnings with `IWYU pragma: export` or `begin_exports` +
`end_exports` until we have support for these pragmas. There are too many
false-positive warnings for the headers that have the correct pragmas for now
and it makes the user experience very unpleasant.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125468

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -544,8 +544,7 @@
   EXPECT_TRUE(
   ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  auto Unused = computeUnusedIncludes(AST);
-  EXPECT_THAT(Unused, IsEmpty());
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
 TEST(IncludeCleaner, RecursiveInclusion) {
@@ -576,8 +575,35 @@
   findReferencedLocations(AST), AST.getIncludeStructure(),
   AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  auto Unused = computeUnusedIncludes(AST);
-  EXPECT_THAT(Unused, IsEmpty());
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+}
+
+TEST(IncludeCleaner, IWYUPragmaExport) {
+  TestTU TU;
+  TU.Code = R"cpp(
+#include "foo.h"
+)cpp";
+  TU.AdditionalFiles["foo.h"] = R"cpp(
+#ifndef FOO_H
+#define FOO_H
+
+#include "bar.h" // IWYU pragma: export
+
+#endif
+  )cpp";
+  TU.AdditionalFiles["bar.h"] = guard(R"cpp(
+void bar() {}
+  )cpp");
+  ParsedAST AST = TU.build();
+
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
+  EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+  // FIXME: This is not correct: foo.h and bar.h are unused but are not
+  // diagnosed as such because we ignore headers with IWYU export pragmas for
+  // now.
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -406,7 +406,7 @@
   #define RECURSIVE_H
 
   #include "recursive.h"
-  
+
   #endif // RECURSIVE_H
 )cpp";
 
@@ -418,6 +418,32 @@
   EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes)));
 }
 
+TEST_F(HeadersTest, HasIWYUPragmas) {
+  FS.Files[MainFile] = R"cpp(
+#include "export.h"
+#include "begin_exports.h"
+#include "none.h"
+)cpp";
+  FS.Files[testPath("export.h")] = R"cpp(
+#pragma once
+#include "none.h" // IWYU pragma: export
+)cpp";
+  FS.Files[testPath("begin_exports.h")] = R"cpp(
+#pragma once
+// IWYU pragma: begin_exports
+#include "none.h"
+// IWYU pragma: end_exports
+)cpp";
+  FS.Files[testPath("none.h")] = R"cpp(
+#pragma once
+)cpp";
+
+  auto Includes = collectIncludes();
+  EXPECT_TRUE(Includes.hasIWYUPragmas(getID("export.h", Includes)));
+  EXPECT_TRUE(Includes.hasIWYUPragmas(getID("begin_exports.h", Includes)));
+  EXPECT_FALSE(Includes.hasIWYUPragmas(getID("none.h", Includes)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -82,7 +82,7 @@
 // Using templateName case is handled by the override TraverseTemplateName.
 if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate)
   return true;
-add(TST->getAsCXXRecordDecl());  // Specialization
+add(TST->getAsCXXRecordDecl()); // Specialization
 return true;
   }
 
@@ -271,9 +271,13 @@
   // Headers without include guards have side effects and are not
   // self-contained, skip them.
   assert(Inc.HeaderID);
+  auto HID = static_cast(*Inc.HeaderID);
+  // FIXME: Ignore the headers with IWYU export pragmas for now, remove this
+  // check when we have actual support for export pragmas.
+  if (AST.getIncludeStructure().hasIWYUPragmas(HID))
+return false;
   auto FE = AST.getSourceManager().getFileManager().getFileRef(
-  AST.getIncludeSt

[PATCH] D125468: [clangd] Include Cleaner: ignore headers with IWYU export pragmas

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 428947.
kbobyrev added a comment.

Remove unwanted formatting changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125468

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -544,8 +544,7 @@
   EXPECT_TRUE(
   ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  auto Unused = computeUnusedIncludes(AST);
-  EXPECT_THAT(Unused, IsEmpty());
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
 TEST(IncludeCleaner, RecursiveInclusion) {
@@ -576,8 +575,35 @@
   findReferencedLocations(AST), AST.getIncludeStructure(),
   AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  auto Unused = computeUnusedIncludes(AST);
-  EXPECT_THAT(Unused, IsEmpty());
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+}
+
+TEST(IncludeCleaner, IWYUPragmaExport) {
+  TestTU TU;
+  TU.Code = R"cpp(
+#include "foo.h"
+)cpp";
+  TU.AdditionalFiles["foo.h"] = R"cpp(
+#ifndef FOO_H
+#define FOO_H
+
+#include "bar.h" // IWYU pragma: export
+
+#endif
+  )cpp";
+  TU.AdditionalFiles["bar.h"] = guard(R"cpp(
+void bar() {}
+  )cpp");
+  ParsedAST AST = TU.build();
+
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
+  EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+  // FIXME: This is not correct: foo.h and bar.h are unused but are not
+  // diagnosed as such because we ignore headers with IWYU export pragmas for
+  // now.
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -406,7 +406,7 @@
   #define RECURSIVE_H
 
   #include "recursive.h"
-  
+
   #endif // RECURSIVE_H
 )cpp";
 
@@ -418,6 +418,32 @@
   EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes)));
 }
 
+TEST_F(HeadersTest, HasIWYUPragmas) {
+  FS.Files[MainFile] = R"cpp(
+#include "export.h"
+#include "begin_exports.h"
+#include "none.h"
+)cpp";
+  FS.Files[testPath("export.h")] = R"cpp(
+#pragma once
+#include "none.h" // IWYU pragma: export
+)cpp";
+  FS.Files[testPath("begin_exports.h")] = R"cpp(
+#pragma once
+// IWYU pragma: begin_exports
+#include "none.h"
+// IWYU pragma: end_exports
+)cpp";
+  FS.Files[testPath("none.h")] = R"cpp(
+#pragma once
+)cpp";
+
+  auto Includes = collectIncludes();
+  EXPECT_TRUE(Includes.hasIWYUPragmas(getID("export.h", Includes)));
+  EXPECT_TRUE(Includes.hasIWYUPragmas(getID("begin_exports.h", Includes)));
+  EXPECT_FALSE(Includes.hasIWYUPragmas(getID("none.h", Includes)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -271,9 +271,13 @@
   // Headers without include guards have side effects and are not
   // self-contained, skip them.
   assert(Inc.HeaderID);
+  auto HID = static_cast(*Inc.HeaderID);
+  // FIXME: Ignore the headers with IWYU export pragmas for now, remove this
+  // check when we have actual support for export pragmas.
+  if (AST.getIncludeStructure().hasIWYUPragmas(HID))
+return false;
   auto FE = AST.getSourceManager().getFileManager().getFileRef(
-  AST.getIncludeStructure().getRealPath(
-  static_cast(*Inc.HeaderID)));
+  AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
   if (!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
   &FE->getFileEntry())) {
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -145,6 +145,10 @@
 return !NonSelfContained.contains(ID);
   }
 
+  bool hasIWYUPragmas(HeaderID ID) const {
+return HasIWYUPragmas.contains(ID);
+  }
+
   // Return all transitively reachable files.
   llvm::ArrayRef allHeaders() const { return RealPathNames; }
 
@@ -185,6 +189,9 @@
   // Contain

[PATCH] D125468: [clangd] Include Cleaner: ignore headers with IWYU export pragmas

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 428949.
kbobyrev added a comment.

Remove redundant comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125468

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -544,8 +544,7 @@
   EXPECT_TRUE(
   ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  auto Unused = computeUnusedIncludes(AST);
-  EXPECT_THAT(Unused, IsEmpty());
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
 TEST(IncludeCleaner, RecursiveInclusion) {
@@ -576,8 +575,35 @@
   findReferencedLocations(AST), AST.getIncludeStructure(),
   AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  auto Unused = computeUnusedIncludes(AST);
-  EXPECT_THAT(Unused, IsEmpty());
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+}
+
+TEST(IncludeCleaner, IWYUPragmaExport) {
+  TestTU TU;
+  TU.Code = R"cpp(
+#include "foo.h"
+)cpp";
+  TU.AdditionalFiles["foo.h"] = R"cpp(
+#ifndef FOO_H
+#define FOO_H
+
+#include "bar.h" // IWYU pragma: export
+
+#endif
+  )cpp";
+  TU.AdditionalFiles["bar.h"] = guard(R"cpp(
+void bar() {}
+  )cpp");
+  ParsedAST AST = TU.build();
+
+  auto ReferencedFiles = findReferencedFiles(
+  findReferencedLocations(AST), AST.getIncludeStructure(),
+  AST.getCanonicalIncludes(), AST.getSourceManager());
+  EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+  // FIXME: This is not correct: foo.h and bar.h are unused but are not
+  // diagnosed as such because we ignore headers with IWYU export pragmas for
+  // now.
+  EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -406,7 +406,7 @@
   #define RECURSIVE_H
 
   #include "recursive.h"
-  
+
   #endif // RECURSIVE_H
 )cpp";
 
@@ -418,6 +418,32 @@
   EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes)));
 }
 
+TEST_F(HeadersTest, HasIWYUPragmas) {
+  FS.Files[MainFile] = R"cpp(
+#include "export.h"
+#include "begin_exports.h"
+#include "none.h"
+)cpp";
+  FS.Files[testPath("export.h")] = R"cpp(
+#pragma once
+#include "none.h" // IWYU pragma: export
+)cpp";
+  FS.Files[testPath("begin_exports.h")] = R"cpp(
+#pragma once
+// IWYU pragma: begin_exports
+#include "none.h"
+// IWYU pragma: end_exports
+)cpp";
+  FS.Files[testPath("none.h")] = R"cpp(
+#pragma once
+)cpp";
+
+  auto Includes = collectIncludes();
+  EXPECT_TRUE(Includes.hasIWYUPragmas(getID("export.h", Includes)));
+  EXPECT_TRUE(Includes.hasIWYUPragmas(getID("begin_exports.h", Includes)));
+  EXPECT_FALSE(Includes.hasIWYUPragmas(getID("none.h", Includes)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -271,9 +271,13 @@
   // Headers without include guards have side effects and are not
   // self-contained, skip them.
   assert(Inc.HeaderID);
+  auto HID = static_cast(*Inc.HeaderID);
+  // FIXME: Ignore the headers with IWYU export pragmas for now, remove this
+  // check when we have actual support for export pragmas.
+  if (AST.getIncludeStructure().hasIWYUPragmas(HID))
+return false;
   auto FE = AST.getSourceManager().getFileManager().getFileRef(
-  AST.getIncludeStructure().getRealPath(
-  static_cast(*Inc.HeaderID)));
+  AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
   if (!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
   &FE->getFileEntry())) {
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -145,6 +145,10 @@
 return !NonSelfContained.contains(ID);
   }
 
+  bool hasIWYUPragmas(HeaderID ID) const {
+return HasIWYUPragmas.contains(ID);
+  }
+
   // Return all transitively reachable files.
   llvm::ArrayRef allHeaders() const { return RealPathNames; }
 
@@ -185,6 +189,9 @@
   // Contains HeaderID

[PATCH] D124702: [MSVC] Add support for pragma function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thanks for all the work here!




Comment at: clang/docs/ReleaseNotes.rst:272
 
+- Added support for MSVC's pragma function, which tells the compiler to
+  generate calls to functions listed in the pragma instead of using the




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D125383#3509023 , @whisperity 
wrote:

> In D125383#3508960 , @aaron.ballman 
> wrote:
>
>> Do you expect to use this matcher in a new check in the immediate future?
>
> D124446  would like to use it.

Thanks! Then I think the only thing missing here are updates to 
https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Registry.cpp.

LGTM with that change made.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125383

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

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Oh, you should probably also add a release note for the new matcher to 
clang/docs/ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125383

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

[PATCH] D121911: [Clang] Add DriverKit support

[Alex Lorenz via Phabricator via cfe-commits]

arphaman accepted this revision.
arphaman added a subscriber: NoQ.
arphaman added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: MaskRay.

LGTM, with one request.

Can you please take the static analyzer changes (RetainSummaryManager.h/cpp) 
and make them into a separate patch? Please add @NoQ as a reviewer for that 
change.

Then feel free to land the remaining driver kit changes in this patch as one 
commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121911

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

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added a comment.

In D122734#3508294 , @uabelho wrote:

> Hi,
>
> I noticed when compiling with gcc 9.3.0 that we get a bunch of new warnings 
> with this patch:
>
>   [1/351] Building CXX object 
> tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/MicrosoftCXXABI.cpp.o
>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:57:12: warning: 'virtual unsigned 
> int {anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
> clang::VarDecl*, unsigned int)' was hidden [-Woverloaded-virtual]
>  57 |   unsigned getManglingNumber(const VarDecl *VD,
> |^
>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
> unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
> clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
>  80 |   unsigned getManglingNumber(const TagDecl *TD,
> |^
>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:46:12: warning: 'virtual unsigned 
> int {anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
> clang::BlockDecl*)' was hidden [-Woverloaded-virtual]
>  46 |   unsigned getManglingNumber(const BlockDecl *BD) override {
> |^
>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
> unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
> clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
>  80 |   unsigned getManglingNumber(const TagDecl *TD,
> |^
>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:42:12: warning: 'virtual unsigned 
> int {anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
> clang::CXXMethodDecl*)' was hidden [-Woverloaded-virtual]
>  42 |   unsigned getManglingNumber(const CXXMethodDecl *CallOperator) 
> override {
> |^
>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
> unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
> clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
>  80 |   unsigned getManglingNumber(const TagDecl *TD,
> |^
>
> No idea if it's important or if gcc is overly picky.

Thanks for letting me know. This is due to the newly added function hiding 
other overloaded functions in the parent class. I will fix it by adding using 
MicrosoftNumberingContext::getManglingNumber to MSHIPNumberingContext.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122734

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM as well, thank you for this!


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

https://reviews.llvm.org/D124996

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

[PATCH] D125401: [OpenCL] Do not guard vload/store_half builtins

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh updated this revision to Diff 428952.
svenvh added a comment.

Add test case.


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

https://reviews.llvm.org/D125401

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/half.cl

Index: clang/test/SemaOpenCL/half.cl
===
--- clang/test/SemaOpenCL/half.cl
+++ clang/test/SemaOpenCL/half.cl
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -Wno-unused-value -triple spir-unknown-unknown
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -Wno-unused-value -triple spir-unknown-unknown -fdeclare-opencl-builtins -finclude-default-header
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -Wno-unused-value -triple spir-unknown-unknown -fdeclare-opencl-builtins -finclude-default-header -DHAVE_BUILTINS
 
 constant float f = 1.0h; // expected-error{{half precision constant requires cl_khr_fp16}}
 
@@ -22,6 +22,11 @@
   half *allowed2 = &*p;
   half *allowed3 = p + 1;
 
+#ifdef HAVE_BUILTINS
+  (void)ilogb(*p); // expected-error{{loading directly from pointer to type '__private half' requires cl_khr_fp16. Use vector data load builtin functions instead}}
+  vstore_half(42.0f, 0, p);
+#endif
+
   return h;
 }
 
@@ -49,6 +54,11 @@
   half *allowed2 = &*p;
   half *allowed3 = p + 1;
 
+#ifdef HAVE_BUILTINS
+  (void)ilogb(*p);
+  vstore_half(42.0f, 0, p);
+#endif
+
   return h;
 }
 
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -352,9 +352,22 @@
 let Extension = Fp64TypeExt in {
   def Double: Type<"double",QualType<"Context.DoubleTy">>;
 }
+
+// The half type for builtins that require the cl_khr_fp16 extension.
 let Extension = Fp16TypeExt in {
   def Half  : Type<"half",  QualType<"Context.HalfTy">>;
 }
+
+// Without the cl_khr_fp16 extension, the half type can only be used to declare
+// a pointer.  Define const and non-const pointer types in all address spaces.
+// Use the "__half" alias to allow the TableGen emitter to distinguish the
+// (extensionless) pointee type of these pointer-to-half types from the "half"
+// type defined above that already carries the cl_khr_fp16 extension.
+foreach AS = [PrivateAS, GlobalAS, ConstantAS, LocalAS, GenericAS] in {
+  def "HalfPtr" # AS  : PointerType>, AS>;
+  def "HalfPtrConst" # AS : PointerType>>, AS>;
+}
+
 def Size  : Type<"size_t",QualType<"Context.getSizeType()">>;
 def PtrDiff   : Type<"ptrdiff_t", QualType<"Context.getPointerDiffType()">>;
 def IntPtr: Type<"intptr_t",  QualType<"Context.getIntPtrType()">>;
@@ -877,22 +890,22 @@
 
 multiclass VloadVstoreHalf addrspaces, bit defStores> {
   foreach AS = addrspaces in {
-def : Builtin<"vload_half", [Float, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin<"vload_half", [Float, Size, !cast("HalfPtrConst" # AS)], Attr.Pure>;
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload_half" # VSize, "vloada_half" # VSize] in {
-def : Builtin, Size, PointerType, AS>], Attr.Pure>;
+def : Builtin, Size, !cast("HalfPtrConst" # AS)], Attr.Pure>;
   }
 }
 if defStores then {
   foreach rnd = ["", "_rte", "_rtz", "_rtp", "_rtn"] in {
 foreach name = ["vstore_half" # rnd] in {
-  def : Builtin]>;
-  def : Builtin]>;
+  def : Builtin("HalfPtr" # AS)]>;
+  def : Builtin("HalfPtr" # AS)]>;
 }
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vstore_half" # VSize # rnd, "vstorea_half" # VSize # rnd] in {
-def : Builtin, Size, PointerType]>;
-def : Builtin, Size, PointerType]>;
+def : Builtin, Size, !cast("HalfPtr" # AS)]>;
+def : Builtin, Size, !cast("HalfPtr" # AS)]>;
   }
 }
   }
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -202,6 +202,9 @@
 typedef double double16 __attribute__((ext_vector_type(16)));
 #endif
 
+// An internal alias for half, for use by OpenCLBuiltins.td.
+#define __half half
+
 #if defined(__OPENCL_CPP_VERSION__)
 #define NULL nullptr
 #elif defined(__OPENCL_C_VERSION__)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Abid via Phabricator via cfe-commits]

abidmalikwaterloo updated this revision to Diff 428955.
abidmalikwaterloo added a comment.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Added the assembly format, dist_schedule parser support


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

Files:
  clang/lib/Testing/CMakeLists.txt
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -207,6 +207,58 @@
   return success();
 }
 
+//===-===//
+// Parser for Distribute Schedule Clause 
+//===-===//
+
+static ParseResult
+parseDistributeScheduleClause(
+OpAsmParser &parser, ClauseScheduleKindAttr &scheduleAttr,
+Optional &chunkSize, Type &chunkType) {
+
+StringRef keyword;
+if (parser.parseKeyword(&keyword))
+	return failure();
+llvm::Optional schedule =
+	symbolizeClauseScheduleKind(keyword);
+if (!schedule)
+	return parser.emitError(parser.getNameLoc()) <<
+" expected scheudle kind";
+
+scheduleAttr = ClauseScheduleKindAttr::get(parser.getContext(), *schedule);
+
+switch (*schedule) {
+case ClauseScheduleKind::Static:
+case ClauseScheduleKind::Dynamic:
+case ClauseScheduleKind::Guided:
+  if (succeeded(parser.parseOptionalEqual())) {
+  	chunkSize = OpAsmParser::UnresolvedOperand{};
+  	if (parser.parseOperand(*chunkSize) || 
+  	parser.parseColonType(chunkType))
+  return failure();
+	} else {
+  	  chunkSize = llvm::NoneType::None;
+	}		
+	break;
+case ClauseScheduleKind::Auto:
+case ClauseScheduleKind::Runtime:
+  chunkSize = llvm::NoneType::None;
+}	
+		
+return success();
+}
+
+// Print distribute schedule clause
+static void printDistributeScheduleClause(OpAsmPrinter &p, Operation *op,
+ClauseScheduleKindAttr schedAttr,
+Value scheduleChunkVar,
+Type scheduleChunkType) {
+p << stringifyClauseScheduleKind(schedAttr.getValue());
+if (scheduleChunkVar)
+  p << " = " << scheduleChunkVar << " : " << scheduleChunkVar.getType();
+  
+}
+
 /// schedule ::= `schedule` `(` sched-list `)`
 /// sched-list ::= sched-val | sched-val sched-list |
 ///sched-val `,` sched-modifier
@@ -517,7 +569,7 @@
 /// loop-bounds := `(` ssa-id-list `)` to `(` ssa-id-list `)` inclusive? steps
 /// steps := `step` `(`ssa-id-list`)`
 ParseResult
-parseWsLoopControl(OpAsmParser &parser, Region ®ion,
+parseLoopControl(OpAsmParser &parser, Region ®ion,
SmallVectorImpl &lowerBound,
SmallVectorImpl &upperBound,
SmallVectorImpl &steps,
@@ -560,7 +612,7 @@
   return success();
 }
 
-void printWsLoopControl(OpAsmPrinter &p, Operation *op, Region ®ion,
+void printLoopControl(OpAsmPrinter &p, Operation *op, Region ®ion,
 ValueRange lowerBound, ValueRange upperBound,
 ValueRange steps, TypeRange loopVarTypes,
 UnitAttr inclusive) {
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -394,7 +394,7 @@
   custom(
 $reduction_vars, type($reduction_vars), $reductions
   ) `)`
-) `for` custom($region, $lowerBound, $upperBound, $step,
+) `for` custom($region, $lowerBound, $upperBound, $step,
   type($step), $inclusive) attr-dict
   }];
   let hasVerifier = 1;
@@ -463,6 +463,71 @@
   let assemblyFormat = [{ ( `(` $results^ `:` type($results) `)` )? attr-dict}];
 }
 
+//===--===//
+// 2.9.4.1 distribute Construct
+//===--===//
+
+def DistributeOp : OpenMP_Op<"distribute", [AttrSizedOperandSegments,
+AllTypesMatch<["lowerBound", "upperBound", "step"]>]> {
+  let summary = "distribute loop construct";
+  let description = [{ 
+The distribute construct specifies that the iterations of one or more loop
+will be executed by the initial teams in the context of their implicit 
+tasks. The iterations are distributed across the initial threads of all 
+initial teams that execute the teams region to which the distribute region 
+binds.
+
+The distribute loop construct specifies 

[PATCH] D122126: [LoopVectorize] Don't interleave when the number of runtime checks exceeds the threshold

[Florian Hahn via Phabricator via cfe-commits]

fhahn accepted this revision.
fhahn added a comment.

LGTM with additional suggestions inline, thanks!




Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10462
+} else {
+  ORE->emit([&]() {
+return OptimizationRemarkAnalysisAliasing(

I think we usually try to use early exits to reduce the indentation, so it 
might be worth doing something like 

```
if (LVP.requiresTooManyRuntimeChecks()) {
 ORE->emit([&]() {
return OptimizationRemarkAnalysisAliasing(
   DEBUG_TYPE, "CantReorderMemOps", L->getStartLoc(),
   L->getHeader())
   << "loop not vectorized: cannot prove it is safe to reorder "
  "memory operations";
  });
  LLVM_DEBUG(dbgs() << "LV: Too many memory checks needed.\n");
  Hints.emitRemarkWithHints();
  return false;
}

// Select the interleave count.
 IC = CM.selectInterleaveCount(VF.Width, *VF.Cost.getValue());
```

(this has the added benefit of not checking for 
`!LVP.requiresTooManyRuntimeChecks()` but the unnegated version, which is 
slightly more straight forward)



Comment at: 
llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll:3
+ 
+; The case will do aggressive interleave on PowerPC, resulting in a lot of 
memory checks.
+; (On the A2, always unroll aggressively. In fact, if aggressive interleaving 
is enabled,

Might be good to precommit the test case and then just show the difference in 
this diff (without the fix `; CHECK: vector.memcheck`) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122126

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

[PATCH] D124860: [clang][AArch64][SVE] Implicit conversions for vector-scalar operations

[David Truby via Phabricator via cfe-commits]

DavidTruby added a comment.

I think this needs more tests to ensure every diagnostic is being triggered 
correctly so I will be adding those in another patch, but I wanted to put up 
the fix for the original issue as soon as possible!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124860

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

[clang] 9766fed - [DeadArgElim] Re-apply: Set unused arguments for internal functions

[Quentin Colombet via cfe-commits]

Author: Quentin Colombet
Date: 2022-05-12T08:46:16-07:00
New Revision: 9766fed9c10e8ba2f67fad0a3e8b509a8064f7b3

URL: 
https://github.com/llvm/llvm-project/commit/9766fed9c10e8ba2f67fad0a3e8b509a8064f7b3
DIFF: 
https://github.com/llvm/llvm-project/commit/9766fed9c10e8ba2f67fad0a3e8b509a8064f7b3.diff

LOG: [DeadArgElim] Re-apply: Set unused arguments for internal functions

The re-apply includes fixes to clang tests that were missed in
the original commit.

Original message:
Prior to this patch we would only set to undef the unused arguments of the
external functions. The rationale was that unused arguments of internal
functions wouldn't need to be turned into undef arguments because they
should have been simply eliminated by the time we reach that code.

This is actually not true because there are plenty of cases where we can't
remove unused arguments. For instance, if the internal function is used in
an indirect call, it may not be possible to change the function signature.
Yet, for statically known call-sites we would still like to mark the unused
arguments as undef.

This patch enables the "set undef arguments" optimization on internal
functions when we encounter cases where internal functions cannot be
optimized. I.e., whenever an internal function is marked "live".

Differential Revision: https://reviews.llvm.org/D124699

Added: 
llvm/test/Transforms/DeadArgElim/fct_ptr.ll

Modified: 
clang/test/CodeGen/debug-info-block-vars.c
clang/test/CodeGenObjCXX/nrvo.mm
llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

Removed: 




diff  --git a/clang/test/CodeGen/debug-info-block-vars.c 
b/clang/test/CodeGen/debug-info-block-vars.c
index dc522a807951..11c899fa3c81 100644
--- a/clang/test/CodeGen/debug-info-block-vars.c
+++ b/clang/test/CodeGen/debug-info-block-vars.c
@@ -11,7 +11,10 @@
 // CHECK: call void @llvm.dbg.declare(metadata i8** %.block_descriptor.addr,
 // CHECK-SAME:metadata !DIExpression())
 // CHECK-OPT-NOT: alloca
-// CHECK-OPT: call void @llvm.dbg.value(metadata i8* %.block_descriptor,
+// Since the block address is not used anywhere in this function,
+// the optimizer (DeadArgElim) has replaced all the false uses
+// (i.e., metadata users) with undef.
+// CHECK-OPT: call void @llvm.dbg.value(metadata i8* undef,
 // CHECK-OPT-SAME:  metadata !DIExpression())
 void f(void) {
   a(^{

diff  --git a/clang/test/CodeGenObjCXX/nrvo.mm 
b/clang/test/CodeGenObjCXX/nrvo.mm
index 89d9ae9639cc..0e4b98996965 100644
--- a/clang/test/CodeGenObjCXX/nrvo.mm
+++ b/clang/test/CodeGenObjCXX/nrvo.mm
@@ -22,7 +22,11 @@ - (X)getNRVO {
 
 X blocksNRVO() {
   return ^{
-// CHECK-LABEL: define internal void @___Z10blocksNRVOv_block_invoke
+// With the optimizer enabled, the DeadArgElim pass is able to
+// mark the block litteral address argument as unused and later the
+// related block_litteral global variable is removed.
+// This allows to promote this call to a fastcc call.
+// CHECK-LABEL: define internal fastcc void @___Z10blocksNRVOv_block_invoke
 X x;
 // CHECK: call void @_ZN1XC1Ev
 // CHECK-NEXT: ret void

diff  --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp 
b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
index 95f6a4f4fb57..a879a0fb30b3 100644
--- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -268,9 +268,12 @@ bool 
DeadArgumentEliminationPass::RemoveDeadArgumentsFromCallers(Function &Fn) {
   if (!Fn.hasExactDefinition())
 return false;
 
-  // Functions with local linkage should already have been handled, except the
-  // fragile (variadic) ones which we can improve here.
-  if (Fn.hasLocalLinkage() && !Fn.getFunctionType()->isVarArg())
+  // Functions with local linkage should already have been handled, except if
+  // they are fully alive (e.g., called indirectly) and except for the fragile
+  // (variadic) ones. In these cases, we may still be able to improve their
+  // statically known call sites.
+  if ((Fn.hasLocalLinkage() && !LiveFunctions.count(&Fn)) &&
+  !Fn.getFunctionType()->isVarArg())
 return false;
 
   // Don't touch naked functions. The assembly might be using an argument, or

diff  --git a/llvm/test/Transforms/DeadArgElim/fct_ptr.ll 
b/llvm/test/Transforms/DeadArgElim/fct_ptr.ll
new file mode 100644
index ..2e352666c1f6
--- /dev/null
+++ b/llvm/test/Transforms/DeadArgElim/fct_ptr.ll
@@ -0,0 +1,67 @@
+; RUN: opt -S %s -deadargelim -o - | FileCheck %s
+; In that test @internal_fct is used by an instruction
+; we don't know how to rewrite (the comparison that produces
+; %cmp1).
+; Because of that use, we used to bail out on removing the
+; unused arguments for this function.
+; Yet, we should still be able to rewrite the direct calls that are
+; statically known, by replacing the related arguments w

[PATCH] D124699: [DeadArgElim] Set unused arguments for internal functions

[Quentin Colombet 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 rG9766fed9c10e: [DeadArgElim] Re-apply: Set unused arguments 
for internal functions (authored by qcolombet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124699

Files:
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGenObjCXX/nrvo.mm
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/test/Transforms/DeadArgElim/fct_ptr.ll

Index: llvm/test/Transforms/DeadArgElim/fct_ptr.ll
===
--- /dev/null
+++ llvm/test/Transforms/DeadArgElim/fct_ptr.ll
@@ -0,0 +1,67 @@
+; RUN: opt -S %s -deadargelim -o - | FileCheck %s
+; In that test @internal_fct is used by an instruction
+; we don't know how to rewrite (the comparison that produces
+; %cmp1).
+; Because of that use, we used to bail out on removing the
+; unused arguments for this function.
+; Yet, we should still be able to rewrite the direct calls that are
+; statically known, by replacing the related arguments with undef.
+; This is what we check on the call that produces %res2.
+
+define i32 @call_indirect(i32 (i32, i32, i32)* readnone %fct_ptr, i32 %arg1, i32 %arg2, i32 %arg3) {
+; CHECK-LABEL: @call_indirect(
+; CHECK-NEXT:[[CMP0:%.*]] = icmp eq i32 (i32, i32, i32)* [[FCT_PTR:%.*]], @external_fct
+; CHECK-NEXT:br i1 [[CMP0]], label [[CALL_EXT:%.*]], label [[CHK2:%.*]]
+; CHECK:   call_ext:
+; CHECK-NEXT:[[RES1:%.*]] = tail call i32 @external_fct(i32 undef, i32 [[ARG2:%.*]], i32 undef)
+; CHECK-NEXT:br label [[END:%.*]]
+; CHECK:   chk2:
+; CHECK-NEXT:[[CMP1:%.*]] = icmp eq i32 (i32, i32, i32)* [[FCT_PTR]], @internal_fct
+; CHECK-NEXT:br i1 [[CMP1]], label [[CALL_INT:%.*]], label [[CALL_OTHER:%.*]]
+; CHECK:   call_int:
+; CHECK-NEXT:[[RES2:%.*]] = tail call i32 @internal_fct(i32 undef, i32 [[ARG2]], i32 undef)
+; CHECK-NEXT:br label [[END]]
+; CHECK:   call_other:
+; CHECK-NEXT:[[RES3:%.*]] = tail call i32 @other_fct(i32 [[ARG2]])
+; CHECK-NEXT:br label [[END]]
+; CHECK:   end:
+; CHECK-NEXT:[[FINAL_RES:%.*]] = phi i32 [ [[RES1]], [[CALL_EXT]] ], [ [[RES2]], [[CALL_INT]] ], [ [[RES3]], [[CALL_OTHER]] ]
+; CHECK-NEXT:ret i32 [[FINAL_RES]]
+;
+  %cmp0 = icmp eq i32 (i32, i32, i32)* %fct_ptr, @external_fct
+  br i1 %cmp0, label %call_ext, label %chk2
+
+call_ext:
+  %res1 = tail call i32 @external_fct(i32 %arg1, i32 %arg2, i32 %arg3)
+  br label %end
+
+chk2:
+  %cmp1 = icmp eq i32 (i32, i32, i32)* %fct_ptr, @internal_fct
+  br i1 %cmp1, label %call_int, label %call_other
+
+call_int:
+  %res2 = tail call i32 @internal_fct(i32 %arg1, i32 %arg2, i32 %arg3)
+  br label %end
+
+call_other:
+  %res3 = tail call i32 @other_fct(i32 %arg1, i32 %arg2, i32 %arg3)
+  br label %end
+
+end:
+  %final_res = phi i32 [%res1, %call_ext], [%res2, %call_int], [%res3, %call_other]
+  ret i32 %final_res
+}
+
+
+define i32 @external_fct(i32 %unused_arg1, i32 %arg2, i32 %unused_arg3) {
+  ret i32 %arg2
+}
+
+define internal i32 @internal_fct(i32 %unused_arg1, i32 %arg2, i32 %unused_arg3) {
+  ret i32 %arg2
+}
+
+define internal i32 @other_fct(i32 %unused_arg1, i32 %arg2, i32 %unused_arg3) {
+  ret i32 %arg2
+}
+
Index: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
===
--- llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
+++ llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
@@ -268,9 +268,12 @@
   if (!Fn.hasExactDefinition())
 return false;
 
-  // Functions with local linkage should already have been handled, except the
-  // fragile (variadic) ones which we can improve here.
-  if (Fn.hasLocalLinkage() && !Fn.getFunctionType()->isVarArg())
+  // Functions with local linkage should already have been handled, except if
+  // they are fully alive (e.g., called indirectly) and except for the fragile
+  // (variadic) ones. In these cases, we may still be able to improve their
+  // statically known call sites.
+  if ((Fn.hasLocalLinkage() && !LiveFunctions.count(&Fn)) &&
+  !Fn.getFunctionType()->isVarArg())
 return false;
 
   // Don't touch naked functions. The assembly might be using an argument, or
Index: clang/test/CodeGenObjCXX/nrvo.mm
===
--- clang/test/CodeGenObjCXX/nrvo.mm
+++ clang/test/CodeGenObjCXX/nrvo.mm
@@ -22,7 +22,11 @@
 
 X blocksNRVO() {
   return ^{
-// CHECK-LABEL: define internal void @___Z10blocksNRVOv_block_invoke
+// With the optimizer enabled, the DeadArgElim pass is able to
+// mark the block litteral address argument as unused and later the
+// related block_litteral global variable is removed.
+// This allows to promote this call to a fastcc call.
+// CHECK-LABEL: define internal fastcc void @___Z10blocks

[clang] 0f29214 - [clang]Silence warning in MicrosoftCXXABI.cpp

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2022-05-12T12:04:05-04:00
New Revision: 0f292141aadb0489231de31de966c239486e019d

URL: 
https://github.com/llvm/llvm-project/commit/0f292141aadb0489231de31de966c239486e019d
DIFF: 
https://github.com/llvm/llvm-project/commit/0f292141aadb0489231de31de966c239486e019d.diff

LOG: [clang]Silence warning in MicrosoftCXXABI.cpp

Silence warning with gcc 9.3 about:

[1/351] Building CXX object 
tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/MicrosoftCXXABI.cpp.o
../../clang/lib/AST/MicrosoftCXXABI.cpp:57:12: warning: 'virtual unsigned int 
{anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
clang::VarDecl*, unsigned int)' was hidden [-Woverloaded-virtual]
   57 |   unsigned getManglingNumber(const VarDecl *VD,
  |^
../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual unsigned 
int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
   80 |   unsigned getManglingNumber(const TagDecl *TD,
  |^

Change-Id: Ia519e77c6454eb020228478dd6498eaf7864dae8

Added: 


Modified: 
clang/lib/AST/MicrosoftCXXABI.cpp

Removed: 




diff  --git a/clang/lib/AST/MicrosoftCXXABI.cpp 
b/clang/lib/AST/MicrosoftCXXABI.cpp
index b3e26bc889c1..7f4a7b2b9381 100644
--- a/clang/lib/AST/MicrosoftCXXABI.cpp
+++ b/clang/lib/AST/MicrosoftCXXABI.cpp
@@ -69,6 +69,7 @@ class MSHIPNumberingContext : public 
MicrosoftNumberingContext {
   std::unique_ptr DeviceCtx;
 
 public:
+  using MicrosoftNumberingContext::getManglingNumber;
   MSHIPNumberingContext(MangleContext *DeviceMangler) {
 DeviceCtx = createItaniumNumberingContext(DeviceMangler);
   }



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

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D122734#3509096 , @yaxunl wrote:

> In D122734#3508294 , @uabelho wrote:
>
>> Hi,
>>
>> I noticed when compiling with gcc 9.3.0 that we get a bunch of new warnings 
>> with this patch:
>>
>>   [1/351] Building CXX object 
>> tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/MicrosoftCXXABI.cpp.o
>>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:57:12: warning: 'virtual unsigned 
>> int {anonymous}::MicrosoftNumberingContext::getManglingNumber(const 
>> clang::VarDecl*, unsigned int)' was hidden [-Woverloaded-virtual]
>>  57 |   unsigned getManglingNumber(const VarDecl *VD,
>> |^
>>   ../../clang/lib/AST/MicrosoftCXXABI.cpp:80:12: warning:   by 'virtual 
>> unsigned int {anonymous}::MSHIPNumberingContext::getManglingNumber(const 
>> clang::TagDecl*, unsigned int)' [-Woverloaded-virtual]
>>  80 |   unsigned getManglingNumber(const TagDecl *TD,
>> |^

fixed by 0f292141aadb0489231de31de966c239486e019d 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122734

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

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:421
 
 install(
   FILES ${ppc_wrapper_files}

qiongsiwu1 wrote:
> craig.topper wrote:
> > There appear to be two installs of ppc_wrapper_files with different 
> > components. Is that intentional?
> Ah yes this is indeed intentional. `cuda_wrapper_files` and 
> `openmp_wrapper_files` have two install targets as well. The first target is 
> part of the "catch-all" `clang-resource-headers`.  The second targets are not 
> installed by default (`EXCLUDE_FROM_ALL`). and a distribution build can 
> opt-in the relevant set of headers. 
> 
> That said, if there are better ways to implement the logic I am all ears. 
As far as I know, the ppc_wrapper_files are only usable on the powerpc target 
so I don't think they should be treated any different than ppc_files. ppc_files 
only has one install target right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked an inline comment as done.
aaronpuchert added inline comments.



Comment at: clang/include/clang/AST/CommentCommands.td:93
 
+def N  : InlineCommand<"n", 0>;
+

gribozavr2 wrote:
> Could you add a test that shows that the text after \n is not treated as the 
> argument?
> 
> You could verify the comment AST like here: 
> llvm-project/clang/test/AST/ast-dump-comment.cpp
> 
That's a good idea, I'll do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

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

[PATCH] D125429: Comment parsing: Allow inline commands to have 0 or more than 1 argument

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 428968.
aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

Unify `Argument` types and parsing. Add AST test. Simplify TableGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125429

Files:
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/include/clang/AST/CommentParser.h
  clang/include/clang/AST/CommentSema.h
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/AST/ast-dump-comment.cpp
  clang/test/Headers/x86-intrinsics-headers-clean.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1116,49 +1116,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have a valid word argument}}
+// expected-warning@+1 {{'\a' command has no word arguments, expected 1}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+// expected-warning@+1 {{'\anchor' command has no word arguments, expected 1}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have a valid word argument}}
+// expected-warning@+1 {{'@b' command has no word arguments, expected 1}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have a valid word argument}}
+// expected-warning@+1 {{'\c' command has no word arguments, expected 1}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have a valid word argument}}
+// expected-warning@+1 {{'\e' command has no word arguments, expected 1}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have a valid word argument}}
+// expected-warning@+1 {{'\em' command has no word arguments, expected 1}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have a valid word argument}}
+// expected-warning@+1 {{'\p' command has no word arguments, expected 1}}
 /// \p
 int test_inline_no_argument_p_bad(int);
 
Index: clang/test/Headers/x86-intrinsics-headers-clean.cpp
===
--- clang/test/Headers/x86-intrinsics-headers-clean.cpp
+++ clang/test/Headers/x86-intrinsics-headers-clean.cpp
@@ -1,11 +1,11 @@
 // Make sure the intrinsic headers compile cleanly with no warnings or errors.
 
 // RUN: %clang_cc1 -ffreestanding -triple i386-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown \
-// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation \
+// RUN:-Wextra -Werror -Wsystem-headers -Wsign-conversion -Wcast-qual -Wdocumentation -Wdocumentation-pedantic -Wdocumentation-unknown-command \
 // RUN:-fsyntax-only -fretain-comments-from-system-headers -flax-vector-conversions=none -x c++ -verify %s
 
 // expected-no-diagnostics
Index: clang/test/AST/ast-dump-comment.cpp
===
--- clang/test/AST/ast-dump-comment.cpp
+++ clang/test/AST/ast-dump-comment.cpp
@@ -62,6 +62,12 @@
 // CHECK:  VarDecl{{.*}}Test_InlineCommandComment
 // CHECK:InlineCommandComment{{.*}} Name="c" RenderMonospaced Arg[0]="Aaa"
 
+/// \n Aaa
+int Test_InlineCommandComment_NoArgs;
+// CHECK:  VarDecl{{.*}}Test_InlineCommandComment_NoArgs
+// CHECK:InlineCommandComment{{.*}} Name="n" RenderNormal
+// CHECK-NEXT: TextComment{{.*}} Text=" Aaa"
+
 /// \anchor Aaa
 int Test_InlineCommandCommentAnchor;
 // CHECK:  VarDecl{{.*}}Test_InlineCommandComment
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -265,10 +265,8 @@
 // User didn't pro

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 428970.
steplong edited the summary of this revision.
steplong added a comment.

- Added `isPredefinedLibFunction()` check
- Added testcase for __attribute__((no_builtin("*")))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124701

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/no-builtin-2.c

Index: clang/test/CodeGen/no-builtin-2.c
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin-2.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+typedef typeof(sizeof(0)) size_t;
+
+void bar(char *s);
+void *memset(void *s, int c, size_t n);
+void *memcpy(void *d, const void *s, size_t n);
+void *memmove(void *d, const void *s, size_t n);
+
+// CHECK: define{{.*}} void @foo1({{.*}}) #[[NO_NOBUILTIN:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   call void @llvm.memset
+// CHECK:   call void @llvm.memcpy
+// CHECK:   call void @llvm.memmove
+void foo1(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 0, n);
+  memcpy(d, s, n);
+  memmove(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo2({{.*}}) #[[NOBUILTIN_MEMSET:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   call void @llvm.memcpy
+// CHECK:   call void @llvm.memmove
+void foo2(char *s, char *d, size_t n) __attribute__((no_builtin("memset"))) {
+  bar(s);
+  memset(s, 1, n);
+  memcpy(d, s, n);
+  memmove(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo3({{.*}}) #[[NOBUILTIN_MEMSET_MEMCPY:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   {{.*}}call {{.*}} @memcpy
+// CHECK:   call void @llvm.memmove
+void foo3(char *s, char *d, size_t n) __attribute__((no_builtin("memset", "memcpy"))) {
+  bar(s);
+  memset(s, 2, n);
+  memcpy(d, s, n);
+  memmove(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo4({{.*}}) #[[NOBUILTINS:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   {{.*}}call {{.*}} @memcpy
+// CHECK:   {{.*}}call {{.*}} @memmove
+void foo4(char *s, char *d, size_t n) __attribute__((no_builtin("*"))) {
+  bar(s);
+  memset(s, 2, n);
+  memcpy(d, s, n);
+  memmove(s, d, n);
+}
+
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtins"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET_MEMCPY]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTINS]] = {{{.*}}"no-builtins"{{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1142,7 +1142,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, I, BuiltinName, &LiteralLoc))
 return;
 
-  if (Builtin::Context::isBuiltinFunc(BuiltinName))
+  bool ParsedAttrIsWildCard = BuiltinName == kWildcard;
+  if (Builtin::Context::isBuiltinFunc(BuiltinName) || ParsedAttrIsWildCard)
 AddBuiltinName(BuiltinName);
   else
 S.Diag(LiteralLoc, diag::warn_attribute_no_builtin_invalid_builtin_name)
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5034,7 +5034,16 @@
   const FunctionDecl *FD = cast(GD.getDecl());
 
   if (auto builtinID = FD->getBuiltinID()) {
+std::string NoBuiltinFD = ("no-builtin-" + FD->getName()).str();
+std::string NoBuiltins = "no-builtins";
 std::string FDInlineName = (FD->getName() + ".inline").str();
+
+bool IsPredefinedLibFunction =
+CGF.getContext().BuiltinInfo.isPredefinedLibFunction(builtinID);
+bool HasAttributeNoBuiltin =
+CGF.CurFn->getAttributes().hasFnAttr(NoBuiltinFD) ||
+CGF.CurFn->getAttributes().hasFnAttr(NoBuiltins);
+
 // When directing calling an inline builtin, call it through it's mangled
 // name to make it clear it's not the actual builtin.
 if (CGF.CurFn->getName() != FDInlineName &&
@@ -5054,8 +5063,11 @@
 
 // Replaceable builtins provide their own implementation of a builtin. If we
 // are in an inline builtin implementation, avoid trivial infinite
-// recursion.
-else
+// recursion. Honor __attribute__((no_builtin("foo"))) or
+// __attribute((no_builtin("*"))) on the current function unless foo is
+// not a predefined library function which means we must generate the
+// builtin no matter what.
+else if (!IsPredefinedLibFunction || !HasAttributeNoBuiltin)
   return CGCallee::forBuiltin(builtinID, FD

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 428969.
aaron.ballman added a comment.

Rebased.

I removed the test coverage because it turned out to not be testing anything. 
If we exhaust resources, then we don't run the `-verify` to test the diagnostic 
behavior, and we're relying on the crash being the test condition, but any 
crash will suffice.


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

https://reviews.llvm.org/D124915

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp

Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
@@ -2612,6 +2613,19 @@
   return false;
 }
 
+void Parser::warnStackExhausted(SourceLocation Loc) {
+  // Only warn about this once.
+  if (!Actions.WarnedStackExhausted) {
+Diag(Loc, diag::warn_stack_exhausted);
+Actions.WarnedStackExhausted = true;
+  }
+}
+
+void Parser::runWithSufficientStackSpace(SourceLocation Loc,
+ llvm::function_ref Fn) {
+  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+}
+
 bool BalancedDelimiterTracker::diagnoseOverflow() {
   P.Diag(P.Tok, diag::err_bracket_depth_exceeded)
 << P.getLangOpts().BracketDepth;
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5765,11 +5765,12 @@
 }
 
 /// ParseDeclarator - Parse and verify a newly-initialized declarator.
-///
 void Parser::ParseDeclarator(Declarator &D) {
   /// This implements the 'declarator' production in the C grammar, then checks
   /// for well-formedness and issues diagnostics.
-  ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  });
 }
 
 static bool isPtrOperatorToken(tok::TokenKind Kind, const LangOptions &Lang,
@@ -5880,7 +5881,9 @@
   D.ExtendWithDeclSpec(DS);
 
   // Recurse to parse whatever is left.
-  ParseDeclaratorInternal(D, DirectDeclParser);
+  runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, DirectDeclParser);
+  });
 
   // Sema will have to catch (syntactically invalid) pointers into global
   // scope. It has to catch pointers into namespace scope anyway.
@@ -5929,7 +5932,8 @@
 D.ExtendWithDeclSpec(DS);
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+runWithSufficientStackSpace(
+D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
 if (Kind == tok::star)
   // Remember that we parsed a pointer type, and remember the type-quals.
   D.AddTypeInfo(DeclaratorChunk::getPointer(
@@ -5974,7 +5978,8 @@
 }
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+runWithSufficientStackSpace(
+D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
 
 if (D.getNumTypeObjects() > 0) {
   // C++ [dcl.ref]p4: There shall be no references to references.
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -793,6 +793,16 @@
 return PP.LookAhead(N-1);
   }
 
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K is
+  /// guaranteed). Produces a warning if we're low on stack space and allocates
+  /// more in that case. Use this in code that may recurse deeply (for example,
+  /// in template instantiation) to avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,
+   llvm::function_ref Fn);
+
 public:
   /// NextToken - This peeks ahead one token and returns it without
   /// consuming it.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,7 +222,9 @@
   on such language mode, ``-Wpre-c2x-compat`` and ``-Wpre-c++2b-compat``
   diagnostic flags report a compatibility issue.
   Fixes `Issue 55306 `_.
-
+- Clang now checks for stack resource exhaustion when recursively parsing
+  declarators in order to give a diagnostic before we run out of stack space.

[PATCH] D122747: [NFC][ARM] Tests for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 428975.
lenary added a comment.
Herald added subscribers: cfe-commits, MaskRay, hiraditya, mgorny.
Herald added a project: clang.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122747

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -47,6 +47,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_input
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -67,6 +68,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf16
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -87,6 +89,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf32
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -120,6 +123,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -156,6 +161,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -219,6 +227,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB9_2
 ; CHECK-FIX-NEXT:  .LBB9_1: @ =>This Inner Loop Header: Depth=1
@@ -249,6 +259,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_ptr(i8* %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-NOSCHED-LABEL: aese_set8_via_ptr:
 ; CHECK-FIX-NOSCHED:   @ %bb.0:
+; CHECK-FIX-NOSCHED-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NOSCHED-NEXT:ldrb r0, [r0]
 ; CHECK-FIX-NOSCHED-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NOSCHED-NEXT:vmov.8 d0[0], r0
@@ -260,6 +271,7 @@
 ;
 ; CHECK-CORTEX-FIX-LABEL: aese_set8_via_ptr:
 ; CHECK-CORTEX-FIX:   @ %bb.0:
+; CHECK-CORTEX-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-CORTEX-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-CORTEX-FIX-NEXT:ldrb r0, [r0]
 ; CHECK-CORTEX-FIX-NEXT:vmov.8 d0[0], r0
@@ -281,6 +293,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r0
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
@@ -300,6 +313,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_ptr(i1 zeroext %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB12_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
@@ -351,6 +365,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB13_2
@@ -380,6 +395,7 @@
 define arm_aapcs_vfpcc void @aese_set8_loop_via_ptr(i32 %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_loop_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:ldrb r1, [r1]
 ; CHECK-FIX-NEXT:

[clang] 7f9837c - [Headers][MSVC] Define wchar_t in stddef.h like MSVC if not using the builtin type

[Stephen Long via cfe-commits]

Author: Stephen Long
Date: 2022-05-12T09:38:07-07:00
New Revision: 7f9837cfa63663ccd51da3e5de73acec8f776ee8

URL: 
https://github.com/llvm/llvm-project/commit/7f9837cfa63663ccd51da3e5de73acec8f776ee8
DIFF: 
https://github.com/llvm/llvm-project/commit/7f9837cfa63663ccd51da3e5de73acec8f776ee8.diff

LOG: [Headers][MSVC] Define wchar_t in stddef.h like MSVC if not using the 
builtin type

 MSVC expects wchar_t to be defined in stddef.h if /Zc:wchar_t- is specified

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D124026

Added: 
clang/test/Headers/ms-no-wchar.cpp

Modified: 
clang/lib/Headers/stddef.h

Removed: 




diff  --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h
index 15acd4427ca14..a15d21b553174 100644
--- a/clang/lib/Headers/stddef.h
+++ b/clang/lib/Headers/stddef.h
@@ -62,7 +62,7 @@ typedef __SIZE_TYPE__ rsize_t;
 #endif /* defined(__need_STDDEF_H_misc) */
 
 #if defined(__need_wchar_t)
-#ifndef __cplusplus
+#if !defined(__cplusplus) || (defined(_MSC_VER) && !_NATIVE_WCHAR_T_DEFINED)
 /* Always define wchar_t when modules are available. */
 #if !defined(_WCHAR_T) || __has_feature(modules)
 #if !__has_feature(modules)

diff  --git a/clang/test/Headers/ms-no-wchar.cpp 
b/clang/test/Headers/ms-no-wchar.cpp
new file mode 100644
index 0..8fe1dbc4c87f6
--- /dev/null
+++ b/clang/test/Headers/ms-no-wchar.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-windows-msvc 
-fms-compatibility-version=17.00 -fno-wchar %s
+// MSVC defines wchar_t instead of using the builtin if /Zc:wchar_t- is passed
+
+#include 
+
+wchar_t c;



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

[PATCH] D124026: [Headers][MSVC] Define wchar_t in stddef.h like MSVC if not using the builtin type

[Stephen Long via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f9837cfa636: [Headers][MSVC] Define wchar_t in stddef.h 
like MSVC if not using the builtin… (authored by steplong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124026

Files:
  clang/lib/Headers/stddef.h
  clang/test/Headers/ms-no-wchar.cpp


Index: clang/test/Headers/ms-no-wchar.cpp
===
--- /dev/null
+++ clang/test/Headers/ms-no-wchar.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-windows-msvc 
-fms-compatibility-version=17.00 -fno-wchar %s
+// MSVC defines wchar_t instead of using the builtin if /Zc:wchar_t- is passed
+
+#include 
+
+wchar_t c;
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -62,7 +62,7 @@
 #endif /* defined(__need_STDDEF_H_misc) */
 
 #if defined(__need_wchar_t)
-#ifndef __cplusplus
+#if !defined(__cplusplus) || (defined(_MSC_VER) && !_NATIVE_WCHAR_T_DEFINED)
 /* Always define wchar_t when modules are available. */
 #if !defined(_WCHAR_T) || __has_feature(modules)
 #if !__has_feature(modules)


Index: clang/test/Headers/ms-no-wchar.cpp
===
--- /dev/null
+++ clang/test/Headers/ms-no-wchar.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-windows-msvc -fms-compatibility-version=17.00 -fno-wchar %s
+// MSVC defines wchar_t instead of using the builtin if /Zc:wchar_t- is passed
+
+#include 
+
+wchar_t c;
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -62,7 +62,7 @@
 #endif /* defined(__need_STDDEF_H_misc) */
 
 #if defined(__need_wchar_t)
-#ifndef __cplusplus
+#if !defined(__cplusplus) || (defined(_MSC_VER) && !_NATIVE_WCHAR_T_DEFINED)
 /* Always define wchar_t when modules are available. */
 #if !defined(_WCHAR_T) || __has_feature(modules)
 #if !__has_feature(modules)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 428978.
lenary marked 3 inline comments as done.
lenary added a comment.

- Address comment nits
- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-fix-cortex-a57-aes-1742098.c
  llvm/lib/Target/ARM/ARM.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/CMakeLists.txt
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/ARM/aes-erratum-fix.ll

Index: llvm/test/CodeGen/ARM/aes-erratum-fix.ll
===
--- llvm/test/CodeGen/ARM/aes-erratum-fix.ll
+++ llvm/test/CodeGen/ARM/aes-erratum-fix.ll
@@ -47,6 +47,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_input
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -67,6 +68,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf16
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -87,6 +89,7 @@
 ; CHECK-FIX-NEXT:push {r4, lr}
 ; CHECK-FIX-NEXT:mov r4, r0
 ; CHECK-FIX-NEXT:bl get_inputf32
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r4]
 ; CHECK-FIX-NEXT:aese.8 q0, q8
 ; CHECK-FIX-NEXT:aesmc.8 q8, q0
@@ -120,6 +123,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_once_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_once_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q0, q1
 ; CHECK-FIX-NEXT:bx lr
@@ -156,6 +161,9 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_twice_via_val(<16 x i8> %0, <16 x i8> %1) nounwind {
 ; CHECK-FIX-LABEL: aese_twice_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:aese.8 q1, q0
 ; CHECK-FIX-NEXT:aesmc.8 q8, q1
 ; CHECK-FIX-NEXT:aese.8 q8, q0
@@ -219,6 +227,8 @@
 define arm_aapcs_vfpcc <16 x i8> @aese_loop_via_val(i32 %0, <16 x i8> %1, <16 x i8> %2) nounwind {
 ; CHECK-FIX-LABEL: aese_loop_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q1, q1, q1
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB9_2
 ; CHECK-FIX-NEXT:  .LBB9_1: @ =>This Inner Loop Header: Depth=1
@@ -249,6 +259,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_ptr(i8* %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-NOSCHED-LABEL: aese_set8_via_ptr:
 ; CHECK-FIX-NOSCHED:   @ %bb.0:
+; CHECK-FIX-NOSCHED-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NOSCHED-NEXT:ldrb r0, [r0]
 ; CHECK-FIX-NOSCHED-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NOSCHED-NEXT:vmov.8 d0[0], r0
@@ -260,6 +271,7 @@
 ;
 ; CHECK-CORTEX-FIX-LABEL: aese_set8_via_ptr:
 ; CHECK-CORTEX-FIX:   @ %bb.0:
+; CHECK-CORTEX-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-CORTEX-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-CORTEX-FIX-NEXT:ldrb r0, [r0]
 ; CHECK-CORTEX-FIX-NEXT:vmov.8 d0[0], r0
@@ -281,6 +293,7 @@
 define arm_aapcs_vfpcc void @aese_set8_via_val(i8 zeroext %0, <16 x i8> %1, <16 x i8>* %2) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r1]
 ; CHECK-FIX-NEXT:vmov.8 d0[0], r0
 ; CHECK-FIX-NEXT:vmov.8 d16[0], r0
@@ -300,6 +313,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_ptr(i1 zeroext %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB12_2
 ; CHECK-FIX-NEXT:  @ %bb.1:
@@ -351,6 +365,7 @@
 define arm_aapcs_vfpcc void @aese_set8_cond_via_val(i1 zeroext %0, i8 zeroext %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_cond_via_val:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:vld1.64 {d16, d17}, [r2]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:beq .LBB13_2
@@ -380,6 +395,7 @@
 define arm_aapcs_vfpcc void @aese_set8_loop_via_ptr(i32 %0, i8* %1, <16 x i8> %2, <16 x i8>* %3) nounwind {
 ; CHECK-FIX-LABEL: aese_set8_loop_via_ptr:
 ; CHECK-FIX:   @ %bb.0:
+; CHECK-FIX-NEXT:vorr q0, q0, q0
 ; CHECK-FIX-NEXT:ldrb r1, [r1]
 ; CHECK-FIX-NEXT:cmp r0, #0
 ; CHECK-FIX-NEXT:s

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Sam Elliott via Phabricator via cfe-commits]

lenary marked 3 inline comments as done.
lenary added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:12-19
+// The intention is this:
+// - Any 128-bit or 64-bit writes to the neon input register of an AES fused
+//   pair are safe (the inputs are to the AESE/AESD instruction).
+// - Any 32-bit writes to the input register are unsafe, but these may happen
+//   in another function, or only on some control flow paths. In these cases,
+//   conservatively insert the VORRq anyway.
+// - So, analyse both inputs to the AESE/AESD instruction, inserting a VORR if

simon_tatham wrote:
> This description would leave me still confused if I didn't happen to already 
> know roughly what the plan was. It jumps in half way through the explanation 
> that someone would need if they were coming to this pass cold. (E.g. it talks 
> about "the VORRq" before having even mentioned //that// there is one, let 
> alone //why// there is one.)
> 
> How about the suggested text as a rewording?
I have taken this feedback on board, and reworded the explanation based on your 
suggestion, with some slight ordering changes and a little more detail about 
the complex cases and some separation between the erratum itself, and the 
workaround we have chosen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119720

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

[PATCH] D125473: Comment parsing: Treat properties as zero-argument inline commands

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
Herald added a project: All.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

That is more accurate, and using a separate class in TableGen seems
appropriate since these are not parts of the text but properties of the
declaration itself.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125473

Files:
  clang/include/clang/AST/CommentCommands.td


Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -63,6 +63,11 @@
   let IsVerbatimLineCommand = 1;
 }
 
+class PropertyCommand : Command {
+  let NumArgs = 0;
+  let IsInlineCommand = 1;
+}
+
 class DeclarationVerbatimLineCommand :
   VerbatimLineCommand {
   let IsDeclarationCommand = 1;
@@ -275,31 +280,6 @@
 
 def NoOp : VerbatimLineCommand<"noop">;
 
-// These have actually no arguments, but we can treat them as line commands.
-def CallGraph   : VerbatimLineCommand<"callgraph">;
-def HideCallGraph   : VerbatimLineCommand<"hidecallgraph">;
-def CallerGraph : VerbatimLineCommand<"callergraph">;
-def HideCallerGraph : VerbatimLineCommand<"hidecallergraph">;
-def ShowInitializer : VerbatimLineCommand<"showinitializer">;
-def HideInitializer : VerbatimLineCommand<"hideinitializer">;
-def ShowRefBy   : VerbatimLineCommand<"showrefby">;
-def HideRefBy   : VerbatimLineCommand<"hiderefby">;
-def ShowRefs: VerbatimLineCommand<"showrefs">;
-def HideRefs: VerbatimLineCommand<"hiderefs">;
-
-// These also have no argument.
-def Private   : VerbatimLineCommand<"private">;
-def Protected : VerbatimLineCommand<"protected">;
-def Public: VerbatimLineCommand<"public">;
-def Pure  : VerbatimLineCommand<"pure">;
-def Static: VerbatimLineCommand<"static">;
-
-// These also have no argument.
-def NoSubgrouping: VerbatimLineCommand<"nosubgrouping">;
-def PrivateSection   : VerbatimLineCommand<"privatesection">;
-def ProtectedSection : VerbatimLineCommand<"protectedsection">;
-def PublicSection: VerbatimLineCommand<"publicsection">;
-
 // We might also build proper support for if/ifnot/else/elseif/endif.
 def If : VerbatimLineCommand<"if">;
 def IfNot  : VerbatimLineCommand<"ifnot">;
@@ -311,6 +291,32 @@
 def Cond: VerbatimLineCommand<"cond">;
 def EndCond : VerbatimLineCommand<"endcond">;
 
+//===--===//
+// PropertyCommand
+//===--===//
+
+def CallGraph   : PropertyCommand<"callgraph">;
+def HideCallGraph   : PropertyCommand<"hidecallgraph">;
+def CallerGraph : PropertyCommand<"callergraph">;
+def HideCallerGraph : PropertyCommand<"hidecallergraph">;
+def ShowInitializer : PropertyCommand<"showinitializer">;
+def HideInitializer : PropertyCommand<"hideinitializer">;
+def ShowRefBy   : PropertyCommand<"showrefby">;
+def HideRefBy   : PropertyCommand<"hiderefby">;
+def ShowRefs: PropertyCommand<"showrefs">;
+def HideRefs: PropertyCommand<"hiderefs">;
+
+def Private   : PropertyCommand<"private">;
+def Protected : PropertyCommand<"protected">;
+def Public: PropertyCommand<"public">;
+def Pure  : PropertyCommand<"pure">;
+def Static: PropertyCommand<"static">;
+
+def NoSubgrouping: PropertyCommand<"nosubgrouping">;
+def PrivateSection   : PropertyCommand<"privatesection">;
+def ProtectedSection : PropertyCommand<"protectedsection">;
+def PublicSection: PropertyCommand<"publicsection">;
+
 
//===--===//
 // DeclarationVerbatimLineCommand
 
//===--===//


Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -63,6 +63,11 @@
   let IsVerbatimLineCommand = 1;
 }
 
+class PropertyCommand : Command {
+  let NumArgs = 0;
+  let IsInlineCommand = 1;
+}
+
 class DeclarationVerbatimLineCommand :
   VerbatimLineCommand {
   let IsDeclarationCommand = 1;
@@ -275,31 +280,6 @@
 
 def NoOp : VerbatimLineCommand<"noop">;
 
-// These have actually no arguments, but we can treat them as line commands.
-def CallGraph   : VerbatimLineCommand<"callgraph">;
-def HideCallGraph   : VerbatimLineCommand<"hidecallgraph">;
-def CallerGraph : VerbatimLineCommand<"callergraph">;
-def HideCallerGraph : VerbatimLineCommand<"hidecallergraph">;
-def ShowInitializer : VerbatimLineCommand<"showinitializer">;
-def HideInitializer : VerbatimLineCommand<"hideinitializer">;
-def ShowRefBy   : VerbatimLineCommand<"showrefby">;
-def HideRefBy

[PATCH] D124702: [MSVC] Add support for pragma function

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 428981.
steplong added a comment.

- Updated line in docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/pragma-ms-function.c
  clang/test/Preprocessor/pragma_microsoft.c
  clang/test/Preprocessor/pragma_microsoft.cpp

Index: clang/test/Preprocessor/pragma_microsoft.cpp
===
--- clang/test/Preprocessor/pragma_microsoft.cpp
+++ clang/test/Preprocessor/pragma_microsoft.cpp
@@ -1,3 +1,7 @@
 // RUN: %clang_cc1 %s -fsyntax-only -std=c++11 -verify -fms-extensions
 
 #pragma warning(push, 4_D) // expected-warning {{requires a level between 0 and 4}}
+
+extern "C" {
+#pragma function(memset) // no-warning
+}
Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -201,6 +201,27 @@
 #pragma clang diagnostic pop
 #pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
 
+// Test pragma function
+#pragma function(memset) // no-warning
+#pragma function(memcpy, strlen, strlen) // no-warning
+#pragma function()   // no-warning
+#pragma function(asdf)   // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+#pragma function(main)   // expected-warning {{'main' is not a recognized builtin; consider including }}
+#pragma function(// expected-warning {{missing ')' after}}
+#pragma function(int)// expected-warning {{missing ')' after}}
+#pragma function(strcmp) asdf// expected-warning {{extra tokens at end}}
+
+#define __INTRIN_H   // there should be no notes after defining __INTRIN_H
+#pragma function(asdf)   // expected-warning-re {{'asdf' is not a recognized builtin{{$
+#pragma function(memset) // no-warning
+#undef __INTRIN_H
+#pragma function(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+
+// MSVC accepts this, but we decide to reject it based on the MS docs saying the pragma must appear at the global level
+void pragma_function_foo() {
+#pragma function(memset) // expected-error {{'#pragma function' can only appear at file scope}}
+}
+
 #pragma optimize  // expected-warning{{missing '(' after '#pragma optimize'}}
 #pragma optimize( // expected-warning{{expected string literal in '#pragma optimize'}}
 #pragma optimize(a// expected-warning{{expected string literal in '#pragma optimize'}}
Index: clang/test/CodeGen/pragma-ms-function.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-ms-function.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+typedef typeof(sizeof(0)) size_t;
+
+void bar(char *s);
+void *memset(void *s, int c, size_t n);
+void *memcpy(void *d, const void *s, size_t n);
+
+// CHECK: define{{.*}} void @foo1({{.*}}) #[[NO_NOBUILTIN:[0-9]+]]
+void foo1(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 0, n);
+  memcpy(d, s, n);
+}
+
+#pragma function(strlen, memset)
+
+// CHECK: define{{.*}} void @foo2({{.*}}) #[[NOBUILTIN_MEMSET:[0-9]+]]
+void foo2(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 1, n);
+  memcpy(d, s, n);
+}
+
+#pragma function(memcpy)
+
+// CHECK: define{{.*}} void @foo3({{.*}}) #[[NOBUILTIN_MEMSET_MEMCPY:[0-9]+]]
+void foo3(char *s, char *d, size_t n) {
+  bar(s);
+  memset(s, 2, n);
+  memcpy(d, s, n);
+}
+
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET_MEMCPY]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10218,10 +10218,12 @@
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
-  // If this is a function definition, check if we have to apply optnone due to
-  // a pragma.
-  if(D.isFunctionDefinition())
+  // If this is a function definition, check if we have to apply any
+  // attributes (i.e. optnone and no_builtin) due to

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:421
 
 install(
   FILES ${ppc_wrapper_files}

craig.topper wrote:
> qiongsiwu1 wrote:
> > craig.topper wrote:
> > > There appear to be two installs of ppc_wrapper_files with different 
> > > components. Is that intentional?
> > Ah yes this is indeed intentional. `cuda_wrapper_files` and 
> > `openmp_wrapper_files` have two install targets as well. The first target 
> > is part of the "catch-all" `clang-resource-headers`.  The second targets 
> > are not installed by default (`EXCLUDE_FROM_ALL`). and a distribution build 
> > can opt-in the relevant set of headers. 
> > 
> > That said, if there are better ways to implement the logic I am all ears. 
> As far as I know, the ppc_wrapper_files are only usable on the powerpc target 
> so I don't think they should be treated any different than ppc_files. 
> ppc_files only has one install target right?
> As far as I know, the ppc_wrapper_files are only usable on the powerpc target 
Yes that is my understanding as well. 

> ppc_files only has one install target right?
There is indeed one install target specifically setup to include only ppc 
files. Meanwhile, the ppc files are also included in the `${files}` list (which 
is installed as as a component of the catch-all `clang-resource-headers`), so 
the ppc files are included in two different install targets. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think the only thing missing is an update to the documentation: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/AttrDocs.td#L5999,
 and add a release note about the change in functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124701

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

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I don't see how you can test this behavior without figuring out how to get a 
'perfect' number to warn but not crash... The only way to validate that I would 
expect would be to do some bizarre flag that has us just 'don't run' in the 
case where the warning is emitted, but that would be strange/only used for the 
test.

Note that I think you might have an improper comment on the 
`Parser::runWithSufficientStackSpace`, else we perhaps wish to combine the 
Sema.cpp and Parser.cpp implementations in some way.




Comment at: clang/include/clang/Parse/Parser.h:802
+  /// more in that case. Use this in code that may recurse deeply (for example,
+  /// in template instantiation) to avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,

Hmm... The Parser version of this might be used for Template Instantiation?  
Should we remove the SEMA one and move all uses to the Parser then?


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

https://reviews.llvm.org/D124915

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review :)




Comment at: clang/lib/Lex/PPDirectives.cpp:1257
   // If we reached here, the preprocessing token is not valid!
-  Diag(Result, diag::err_pp_invalid_directive);
+  Diag(Result, diag::err_pp_invalid_directive) << 0;
 

aaron.ballman wrote:
> I think we should be attempting to suggest a typo for the error case as well 
> e.g.,
> ```
> #fi WHATEVER
> #endif
> ```
> we currently give no suggestion for that typo, just the error. However, this 
> may require a fair amount of changes because of the various edge cases where 
> we give better diagnostics than "unknown directive". e.g.,
> ```
> #if WHATEVER // error: unterminated conditional directive
> #endfi // no diagnostic
> ```
> so if it looks like covering error cases is going to be involved, I'm fine 
> doing it in a follow-up if you'd prefer.
The former can be implemented easily, but the latter seems not easy.
So what about doing the latter in another patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428983.
ken-matsui added a comment.

Update the code as reviewed and add a release note


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
+
+// pre-c2x-cpp2b-error@+1 {{invalid preprocessing directive, did you mean '#if'?}}
+#id UNDEFINED
+// c2x-cpp2b-error@-1 {{invalid preprocessing directive, did you mean '#if'?}}
+// pre-c2x-cpp2b-error@+1 {{#endif without #if}}
+#endif
+// c2x-cpp2b-error@-1 {{#endif without #if}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector &Candidates) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return SimilarStr->first;
+  } els

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Parse/Parser.cpp:2618
+  // Only warn about this once.
+  if (!Actions.WarnedStackExhausted) {
+Diag(Loc, diag::warn_stack_exhausted);

Another concern here: Do we properly initialize "Actions" in the case where we 
don't have a semantic Action?  That is, if we are just preprocessing?


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

https://reviews.llvm.org/D124915

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 428985.
ken-matsui added a comment.

Add test for errored directive but no suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c

Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s
+// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s
+
+// id:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   not suggested to '#elifdef'
+// elfindef:  not suggested to '#elifdef'
+// elfinndef: not suggested to '#elifndef'
+// els:   pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+// id:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifd:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#if'?}}
+// ifde:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#ifdef'?}}
+// elf:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elsif: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elseif:c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elif'?}}
+// elfidef:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfindef:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifdef'?}}
+// elfinndef: c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#elifndef'?}}
+// els:   c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#else'?}}
+// endi:  c2x-cpp2b-warning@-12 {{invalid preprocessing directive, did you mean '#endif'?}}
+
+#ifdef UNDEFINED
+#i // no diagnostic
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostic
+#endif
+
+// pre-c2x-cpp2b-error@+1 {{invalid preprocessing directive, did you mean '#if'?}}
+#id UNDEFINED
+// c2x-cpp2b-error@-1 {{invalid preprocessing directive, did you mean '#if'?}}
+
+// pre-c2x-cpp2b-error@+1 {{invalid preprocessing directive}}
+#invalid
+// c2x-cpp2b-error@-1 {{invalid preprocessing directive}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -266,6 +266,51 @@
 .Default(false);
 }
 
+/// Find a similar string in `Candidates`.
+///
+/// \param LHS a string for a similar string in `Candidates`
+///
+/// \param Candidates the candidates to find a similar string.
+///
+/// \returns a similar string if exists. If no similar string exists,
+/// returns None.
+static Optional findSimilarStr(
+StringRef LHS, const std::vector &Candidates) {
+  // We need to check if `Candidates` has the exact case-insensitive string
+  // because the Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (LHS.equals_insensitive(C)) {
+  return C;
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If the LHS size is less than 3, use the LHS size minus 1 and if not,
+  // use the LHS size divided by 3.
+  size_t Length = LHS.size();
+  size_t MaxDist = Length < 3 ? Length - 1 : Length / 3;
+
+  Optional> SimilarStr = None;
+  for (StringRef C : Candidates) {
+size_t CurDist = LHS.edit_distance(C, true);
+if (CurDist <= MaxDist) {
+  if (!SimilarStr.hasValue()) {
+// The first similar string found.
+SimilarStr = {C, CurDist};
+  } else if (CurDist < SimilarStr->second) {
+// More similar string found.
+SimilarStr = {C, CurDist};
+  }
+}
+  }
+
+  if (SimilarStr.hasValue()) {
+return

[PATCH] D121733: Clean pathnames in FileManager.

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/lib/Basic/FileManager.cpp:218
+  llvm::sys::path::remove_dots(CleanFilename, /*remove_dot_dot=*/false);
+  Filename = CleanFilename;
+

ppluzhnikov wrote:
> kadircet wrote:
> > this is actually breaking the [contract of 
> > FileEntryRef](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/FileEntry.h#L58-L59),
> >  is this the point?
> > 
> > ```
> > /// A reference to a \c FileEntry that includes the name of the file as it 
> > was
> > /// accessed by the FileManager's client.
> > ```
> > 
> > I don't see mention of this contract being changed explicitly anywhere, if 
> > so can you mention it in the commit message and also update the 
> > documentation? (the same applies to DirectoryEntryRef changes as well)
> > 
> > I am not able to take a detailed look at this at the moment, but this 
> > doesn't feel like a safe change for all clients. Because people might be 
> > relying on this contract without explicitly testing the behaviour for "./" 
> > in the filenames. So tests passing (especially when you're just updating 
> > them to not have `./`) might not be implying it's safe.
> I chased this comment down to commit 4dc5573acc0d2e7c59d8bac2543eb25cb4b32984.
> 
> The commit message says:
> 
>This commit introduces a parallel API to FileManager's getFile: 
> getFileEntryRef, which returns
> a reference to the FileEntry, and the name that was used to access the 
> file. In the case of
> a VFS with 'use-external-names', the FileEntyRef contains the external 
> name of the file,
> not the filename that was used to access it.
> 
> So it appears that the comment itself is not quite correct.
> 
> It's also a bit ambiguous (I think) -- "name used to access the file"
> could be interpreted as the name which clang itself used to access
> the file, and not necessarily the name client used.
> 
> > people might be relying on this contract without explicitly testing the 
> > behaviour for "./" in the filenames.
> 
> That's a possibility.
> 
> I am not sure how to evaluate the benefit of this patch (avoiding noise 
> everywhere) vs. possibly breaking clients.
While the comment is not currently accurate due to use-external-names, [[ 
https://github.com/llvm/llvm-project/blob/47be07074a73bd469b16af440923e3cf3b6b3f10/clang/lib/Basic/FileManager.cpp#L319
 | the goal is to eliminate that behaviour ]] and have a separate API for 
getting the external names.  It's maybe still fine to eliminate `./` here, I 
don't have a strong opinion.

> It's also a bit ambiguous (I think) -- "name used to access the file"
could be interpreted as the name which clang itself used to access
the file, and not necessarily the name client used.

It means the name passed to getFileRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/include/clang/Parse/Parser.h:802
+  /// more in that case. Use this in code that may recurse deeply (for example,
+  /// in template instantiation) to avoid stack overflow.
+  void runWithSufficientStackSpace(SourceLocation Loc,

erichkeane wrote:
> Hmm... The Parser version of this might be used for Template Instantiation?  
> Should we remove the SEMA one and move all uses to the Parser then?
Nope, but I could be less silly and just... use the one from Sema... instead of 
moving it to Parser. Derp!


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

https://reviews.llvm.org/D124915

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

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 428987.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Simplify the implementation by using the facilities Sema already exposes. The 
behavior remains the same:

  F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -cc1 
-fsyntax-only "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:8:1: warning: 
stack nearly exhausted; compilation time may suffer, and crashes due to stack 
overflow are likely [-Wstack-exhausted]
  int PTR6 q3_var = 0;
  ^
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:


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

https://reviews.llvm.org/D124915

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseDecl.cpp


Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5765,11 +5765,12 @@
 }
 
 /// ParseDeclarator - Parse and verify a newly-initialized declarator.
-///
 void Parser::ParseDeclarator(Declarator &D) {
   /// This implements the 'declarator' production in the C grammar, then checks
   /// for well-formedness and issues diagnostics.
-  ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  });
 }
 
 static bool isPtrOperatorToken(tok::TokenKind Kind, const LangOptions &Lang,
@@ -5880,7 +5881,9 @@
   D.ExtendWithDeclSpec(DS);
 
   // Recurse to parse whatever is left.
-  ParseDeclaratorInternal(D, DirectDeclParser);
+  Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, DirectDeclParser);
+  });
 
   // Sema will have to catch (syntactically invalid) pointers into global
   // scope. It has to catch pointers into namespace scope anyway.
@@ -5929,7 +5932,8 @@
 D.ExtendWithDeclSpec(DS);
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+Actions.runWithSufficientStackSpace(
+D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); 
});
 if (Kind == tok::star)
   // Remember that we parsed a pointer type, and remember the type-quals.
   D.AddTypeInfo(DeclaratorChunk::getPointer(
@@ -5974,7 +5978,8 @@
 }
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+Actions.runWithSufficientStackSpace(
+D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); 
});
 
 if (D.getNumTypeObjects() > 0) {
   // C++ [dcl.ref]p4: There shall be no references to references.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,7 +222,9 @@
   on such language mode, ``-Wpre-c2x-compat`` and ``-Wpre-c++2b-compat``
   diagnostic flags report a compatibility issue.
   Fixes `Issue 55306 `_.
-
+- Clang now checks for stack resource exhaustion when recursively parsing
+  declarators in order to give a diagnostic before we run out of stack space.
+  This fixes `Issue 51642 
`_.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5765,11 +5765,12 @@
 }
 
 /// ParseDeclarator - Parse and verify a newly-initialized declarator.
-///
 void Parser::ParseDeclarator(Declarator &D) {
   /// This implements the 'declarator' production in the C grammar, then checks
   /// for well-formedness and issues diagnostics.
-  ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  });
 }
 
 static bool isPtrOperatorToken(tok::TokenKind Kind, const LangOptions &Lang,
@@ -5880,7 +5881,9 @@
   D.ExtendWithDeclSpec(DS);
 
   // Recurse to parse whatever is left.
-  ParseDeclaratorInternal(D, DirectDeclParser);
+  Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, DirectDeclParser);
+  });
 
   // Sema will have to catch (syntactically invalid) pointers into global
   // scope. It has to catch pointers into namespace scope anyway.
@@ -5929,7 +5932,8 @@
 D.ExtendWithDeclSpec(DS);
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+Actions.ru

[PATCH] D122747: [NFC][ARM] Tests for Cortex-A57 and Cortex-A72 Fused AES Erratum

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The test file has 4000+ lines. Any chance making it smaller?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122747

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

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

>> Another concern here: Do we properly initialize "Actions" in the case where 
>> we don't have a semantic Action? That is, if we are just preprocessing?

Doh! Forgot we don't do 'Parser' in preprocessor mode.  LGTM!


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

https://reviews.llvm.org/D124915

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