[PATCH] D50945: [Lex] Make HeaderMaps a unique_ptr vector
MaskRay created this revision.
MaskRay added reviewers: Eugene.Zelenko, dblaikie.
Herald added a subscriber: cfe-commits.
unique_ptr makes the ownership clearer than a raw pointer container.
Repository:
rC Clang
https://reviews.llvm.org/D50945
Files:
include/clang/Lex/HeaderMap.h
include/clang/Lex/HeaderSearch.h
lib/Lex/HeaderMap.cpp
lib/Lex/HeaderSearch.cpp
Index: lib/Lex/HeaderSearch.cpp
===
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -75,12 +75,6 @@
FileMgr(SourceMgr.getFileManager()), FrameworkMap(64),
ModMap(SourceMgr, Diags, LangOpts, Target, *this) {}
-HeaderSearch::~HeaderSearch() {
- // Delete headermaps.
- for (unsigned i = 0, e = HeaderMaps.size(); i != e; ++i)
-delete HeaderMaps[i].second;
-}
-
void HeaderSearch::PrintStats() {
fprintf(stderr, "\n*** HeaderSearch Stats:\n");
fprintf(stderr, "%d files tracked.\n", (int)FileInfo.size());
@@ -113,12 +107,12 @@
// Pointer equality comparison of FileEntries works because they are
// already uniqued by inode.
if (HeaderMaps[i].first == FE)
-return HeaderMaps[i].second;
+return HeaderMaps[i].second.get();
}
- if (const HeaderMap *HM = HeaderMap::Create(FE, FileMgr)) {
-HeaderMaps.push_back(std::make_pair(FE, HM));
-return HM;
+ if (std::unique_ptr HM = HeaderMap::Create(FE, FileMgr)) {
+HeaderMaps.emplace_back(FE, std::move(HM));
+return HeaderMaps.back().second.get();
}
return nullptr;
Index: lib/Lex/HeaderMap.cpp
===
--- lib/Lex/HeaderMap.cpp
+++ lib/Lex/HeaderMap.cpp
@@ -48,7 +48,8 @@
/// map. If it doesn't look like a HeaderMap, it gives up and returns null.
/// If it looks like a HeaderMap but is obviously corrupted, it puts a reason
/// into the string error argument and returns null.
-const HeaderMap *HeaderMap::Create(const FileEntry *FE, FileManager ) {
+std::unique_ptr HeaderMap::Create(const FileEntry *FE,
+ FileManager ) {
// If the file is too small to be a header map, ignore it.
unsigned FileSize = FE->getSize();
if (FileSize <= sizeof(HMapHeader)) return nullptr;
@@ -59,7 +60,7 @@
bool NeedsByteSwap;
if (!checkHeader(**FileBuffer, NeedsByteSwap))
return nullptr;
- return new HeaderMap(std::move(*FileBuffer), NeedsByteSwap);
+ return std::unique_ptr(new HeaderMap(std::move(*FileBuffer), NeedsByteSwap));
}
bool HeaderMapImpl::checkHeader(const llvm::MemoryBuffer ,
Index: include/clang/Lex/HeaderSearch.h
===
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -17,6 +17,7 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/DirectoryLookup.h"
+#include "clang/Lex/HeaderMap.h"
#include "clang/Lex/ModuleMap.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
@@ -38,7 +39,6 @@
class ExternalPreprocessorSource;
class FileEntry;
class FileManager;
-class HeaderMap;
class HeaderSearchOptions;
class IdentifierInfo;
class LangOptions;
@@ -226,9 +226,8 @@
llvm::StringMap;
std::unique_ptr IncludeAliases;
- /// HeaderMaps - This is a mapping from FileEntry -> HeaderMap, uniquing
- /// headermaps. This vector owns the headermap.
- std::vector> HeaderMaps;
+ /// This is a mapping from FileEntry -> HeaderMap, uniquing headermaps.
+ std::vector>> HeaderMaps;
/// The mapping between modules and headers.
mutable ModuleMap ModMap;
@@ -264,7 +263,6 @@
const LangOptions , const TargetInfo *Target);
HeaderSearch(const HeaderSearch &) = delete;
HeaderSearch =(const HeaderSearch &) = delete;
- ~HeaderSearch();
/// Retrieve the header-search options with which this header search
/// was initialized.
Index: include/clang/Lex/HeaderMap.h
===
--- include/clang/Lex/HeaderMap.h
+++ include/clang/Lex/HeaderMap.h
@@ -71,7 +71,8 @@
public:
/// This attempts to load the specified file as a header map. If it doesn't
/// look like a HeaderMap, it gives up and returns null.
- static const HeaderMap *Create(const FileEntry *FE, FileManager );
+ static std::unique_ptr Create(const FileEntry *FE,
+ FileManager );
/// Check to see if the specified relative filename is located in this
/// HeaderMap. If so, open it and return its FileEntry. If RawPath is not
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41284: [Concepts] Associated constraints infrastructure.
saar.raz added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); saar.raz wrote: > Rakete wrote: > > Quuxplusone wrote: > > > saar.raz wrote: > > > > Quuxplusone wrote: > > > > > For my own edification, could you explain whether, given > > > > > > > > > > #define BOOL bool > > > > > using typedef_for_bool = bool; > > > > > > > > > > you'd expect to diagnose a redeclaration of `B::A` with associated > > > > > constraint > > > > > > > > > > requires bool( U() ) // extra whitespace > > > > > > > > > > or > > > > > > > > > > requires BOOL(U()) // different spelling of `bool` > > > > > > > > > > or > > > > > > > > > > requires typedef_for_bool(U()) // different spelling of `bool` > > > > > > > > > > ? My naive reading of N4762 temp.constr.atomic/2 says that none of > > > > > these constraints (on line 10) would be "identical" to the constraint > > > > > on line 6... but then I don't understand what's the salient > > > > > difference between line 10 (which apparently gives no error) and line > > > > > 22 (which apparently gives an error). > > > > Line 22 has a not (!) operator in front of the bool(), I guess you > > > > missed that? > > > I saw the `!`... but I don't understand how the compiler "knows" that > > > `!bool(U())` is "different" from `bool(T())` in a way that doesn't > > > equally apply to `bool(U())`. > > > > > > Or suppose the constraint on line 10 was `requires bool(U())==true`... > > > would that give a diagnostic? > > `bool(T())` and `bool(U())` are identical because they have the same > > parameter mappings. > > > > The "identical" requirement applies to the actual grammar composition of > > the expression, so `bool(T())` would be different to `bool(T()) == true`. > > > > At least that's how I understand it. > OK, I can see where the confusion is coming from. > > The way it works (in clang, at least) - is that the compiler pays no > attention to the name of a template parameter for any purpose other than > actually finding it in the first place - once it is found, it is 'stored' > simply as bool(()) where the first 0 is the depth of > the template parameter list of the parameter in question (in case of a > template within a template) and the second 0 is the index of the template > parameter within that list. > > I believe this treatment stems from [temp.over.link]p6 "When determining > whether types or qualified-concept-names are equivalent, the rules above are > used to compare expressions involving template parameters" Correction - p5 describes this better (see also the example in p4) Repository: rC Clang https://reviews.llvm.org/D41284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41284: [Concepts] Associated constraints infrastructure.
saar.raz added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); Rakete wrote: > Quuxplusone wrote: > > saar.raz wrote: > > > Quuxplusone wrote: > > > > For my own edification, could you explain whether, given > > > > > > > > #define BOOL bool > > > > using typedef_for_bool = bool; > > > > > > > > you'd expect to diagnose a redeclaration of `B::A` with associated > > > > constraint > > > > > > > > requires bool( U() ) // extra whitespace > > > > > > > > or > > > > > > > > requires BOOL(U()) // different spelling of `bool` > > > > > > > > or > > > > > > > > requires typedef_for_bool(U()) // different spelling of `bool` > > > > > > > > ? My naive reading of N4762 temp.constr.atomic/2 says that none of > > > > these constraints (on line 10) would be "identical" to the constraint > > > > on line 6... but then I don't understand what's the salient difference > > > > between line 10 (which apparently gives no error) and line 22 (which > > > > apparently gives an error). > > > Line 22 has a not (!) operator in front of the bool(), I guess you missed > > > that? > > I saw the `!`... but I don't understand how the compiler "knows" that > > `!bool(U())` is "different" from `bool(T())` in a way that doesn't equally > > apply to `bool(U())`. > > > > Or suppose the constraint on line 10 was `requires bool(U())==true`... > > would that give a diagnostic? > `bool(T())` and `bool(U())` are identical because they have the same > parameter mappings. > > The "identical" requirement applies to the actual grammar composition of the > expression, so `bool(T())` would be different to `bool(T()) == true`. > > At least that's how I understand it. OK, I can see where the confusion is coming from. The way it works (in clang, at least) - is that the compiler pays no attention to the name of a template parameter for any purpose other than actually finding it in the first place - once it is found, it is 'stored' simply as bool(()) where the first 0 is the depth of the template parameter list of the parameter in question (in case of a template within a template) and the second 0 is the index of the template parameter within that list. I believe this treatment stems from [temp.over.link]p6 "When determining whether types or qualified-concept-names are equivalent, the rules above are used to compare expressions involving template parameters" Repository: rC Clang https://reviews.llvm.org/D41284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r335081 - Recommit r335063: [Darwin] Add a warning for missing include path for libstdc++
Also, the diag text should probably say "pass '-stdlib=libc++' " not "pass
'-std=libc++' "?
On Sat, Aug 18, 2018 at 11:06 PM Nico Weber wrote:
> Should this maybe not be emitted when compiling e.g. .ii files
> (preprocessed output)? I just got this when I built a standalone .ii file
> with some bug repro after I deleted all -I and -isysroot flags and whatnot,
> since they aren't needed for preprocessed files.
>
> On Tue, Jun 19, 2018 at 6:52 PM Alex Lorenz via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: arphaman
>> Date: Tue Jun 19 15:47:53 2018
>> New Revision: 335081
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=335081=rev
>> Log:
>> Recommit r335063: [Darwin] Add a warning for missing include path for
>> libstdc++
>>
>> The recommit ensures that the tests that failed on bots don't trigger the
>> warning.
>>
>> Xcode 10 removes support for libstdc++, but the users just get a confusing
>> include not file warning when including an STL header (when building for
>> iOS6
>> which uses libstdc++ by default for example).
>> This patch adds a new warning that lets the user know that the libstdc++
>> include
>> path was not found to ensure that the user is more aware of why the error
>> occurs.
>>
>> rdar://40830462
>>
>> Differential Revision: https://reviews.llvm.org/D48297
>>
>> Added:
>> cfe/trunk/test/Frontend/warning-stdlibcxx-darwin.cpp
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
>> cfe/trunk/include/clang/Lex/HeaderSearch.h
>> cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
>> cfe/trunk/test/CodeGenCXX/apple-kext-guard-variable.cpp
>> cfe/trunk/test/Misc/backend-stack-frame-diagnostics.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=335081=335080=335081=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Tue Jun 19
>> 15:47:53 2018
>> @@ -236,4 +236,9 @@ def err_invalid_vfs_overlay : Error<
>>
>> def warn_option_invalid_ocl_version : Warning<
>>"OpenCL version %0 does not support the option '%1'">,
>> InGroup;
>> +
>> +def warn_stdlibcxx_not_found : Warning<
>> + "include path for stdlibc++ headers not found; pass '-std=libc++' on
>> the "
>> + "command line to use the libc++ standard library instead">,
>> + InGroup>;
>> }
>>
>> Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=335081=335080=335081=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
>> +++ cfe/trunk/include/clang/Lex/HeaderSearch.h Tue Jun 19 15:47:53 2018
>> @@ -272,6 +272,8 @@ public:
>>
>>FileManager () const { return FileMgr; }
>>
>> + DiagnosticsEngine () const { return Diags; }
>> +
>>/// Interface for setting the file search paths.
>>void SetSearchPaths(const std::vector ,
>>unsigned angledDirIdx, unsigned systemDirIdx,
>>
>> Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=335081=335080=335081=diff
>>
>> ==
>> --- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)
>> +++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Tue Jun 19 15:47:53 2018
>> @@ -14,6 +14,7 @@
>> #include "clang/Basic/FileManager.h"
>> #include "clang/Basic/LangOptions.h"
>> #include "clang/Config/config.h" // C_INCLUDE_DIRS
>> +#include "clang/Frontend/FrontendDiagnostic.h"
>> #include "clang/Frontend/Utils.h"
>> #include "clang/Lex/HeaderMap.h"
>> #include "clang/Lex/HeaderSearch.h"
>> @@ -55,11 +56,13 @@ public:
>>
>>/// AddPath - Add the specified path to the specified group list,
>> prefixing
>>/// the sysroot if used.
>> - void AddPath(const Twine , IncludeDirGroup Group, bool
>> isFramework);
>> + /// Returns true if the path exists, false if it was ignored.
>> + bool AddPath(const Twine , IncludeDirGroup Group, bool
>> isFramework);
>>
>>/// AddUnmappedPath - Add the specified path to the specified group
>> list,
>>/// without performing any sysroot remapping.
>> - void AddUnmappedPath(const Twine , IncludeDirGroup Group,
>> + /// Returns true if the path exists, false if it was ignored.
>> + bool AddUnmappedPath(const Twine , IncludeDirGroup Group,
>> bool isFramework);
>>
>>/// AddSystemHeaderPrefix - Add the specified prefix to the system
>> header
>> @@ -70,10 +73,9 @@ public:
>>
>>/// AddGnuCPlusPlusIncludePaths - Add the necessary paths to support a
>> gnu
>>/// libstdc++.
>> - void
Re: r335081 - Recommit r335063: [Darwin] Add a warning for missing include path for libstdc++
Should this maybe not be emitted when compiling e.g. .ii files
(preprocessed output)? I just got this when I built a standalone .ii file
with some bug repro after I deleted all -I and -isysroot flags and whatnot,
since they aren't needed for preprocessed files.
On Tue, Jun 19, 2018 at 6:52 PM Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: arphaman
> Date: Tue Jun 19 15:47:53 2018
> New Revision: 335081
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335081=rev
> Log:
> Recommit r335063: [Darwin] Add a warning for missing include path for
> libstdc++
>
> The recommit ensures that the tests that failed on bots don't trigger the
> warning.
>
> Xcode 10 removes support for libstdc++, but the users just get a confusing
> include not file warning when including an STL header (when building for
> iOS6
> which uses libstdc++ by default for example).
> This patch adds a new warning that lets the user know that the libstdc++
> include
> path was not found to ensure that the user is more aware of why the error
> occurs.
>
> rdar://40830462
>
> Differential Revision: https://reviews.llvm.org/D48297
>
> Added:
> cfe/trunk/test/Frontend/warning-stdlibcxx-darwin.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
> cfe/trunk/include/clang/Lex/HeaderSearch.h
> cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
> cfe/trunk/test/CodeGenCXX/apple-kext-guard-variable.cpp
> cfe/trunk/test/Misc/backend-stack-frame-diagnostics.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=335081=335080=335081=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Tue Jun 19
> 15:47:53 2018
> @@ -236,4 +236,9 @@ def err_invalid_vfs_overlay : Error<
>
> def warn_option_invalid_ocl_version : Warning<
>"OpenCL version %0 does not support the option '%1'">,
> InGroup;
> +
> +def warn_stdlibcxx_not_found : Warning<
> + "include path for stdlibc++ headers not found; pass '-std=libc++' on
> the "
> + "command line to use the libc++ standard library instead">,
> + InGroup>;
> }
>
> Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=335081=335080=335081=diff
>
> ==
> --- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
> +++ cfe/trunk/include/clang/Lex/HeaderSearch.h Tue Jun 19 15:47:53 2018
> @@ -272,6 +272,8 @@ public:
>
>FileManager () const { return FileMgr; }
>
> + DiagnosticsEngine () const { return Diags; }
> +
>/// Interface for setting the file search paths.
>void SetSearchPaths(const std::vector ,
>unsigned angledDirIdx, unsigned systemDirIdx,
>
> Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=335081=335080=335081=diff
>
> ==
> --- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)
> +++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Tue Jun 19 15:47:53 2018
> @@ -14,6 +14,7 @@
> #include "clang/Basic/FileManager.h"
> #include "clang/Basic/LangOptions.h"
> #include "clang/Config/config.h" // C_INCLUDE_DIRS
> +#include "clang/Frontend/FrontendDiagnostic.h"
> #include "clang/Frontend/Utils.h"
> #include "clang/Lex/HeaderMap.h"
> #include "clang/Lex/HeaderSearch.h"
> @@ -55,11 +56,13 @@ public:
>
>/// AddPath - Add the specified path to the specified group list,
> prefixing
>/// the sysroot if used.
> - void AddPath(const Twine , IncludeDirGroup Group, bool
> isFramework);
> + /// Returns true if the path exists, false if it was ignored.
> + bool AddPath(const Twine , IncludeDirGroup Group, bool
> isFramework);
>
>/// AddUnmappedPath - Add the specified path to the specified group
> list,
>/// without performing any sysroot remapping.
> - void AddUnmappedPath(const Twine , IncludeDirGroup Group,
> + /// Returns true if the path exists, false if it was ignored.
> + bool AddUnmappedPath(const Twine , IncludeDirGroup Group,
> bool isFramework);
>
>/// AddSystemHeaderPrefix - Add the specified prefix to the system
> header
> @@ -70,10 +73,9 @@ public:
>
>/// AddGnuCPlusPlusIncludePaths - Add the necessary paths to support a
> gnu
>/// libstdc++.
> - void AddGnuCPlusPlusIncludePaths(StringRef Base,
> - StringRef ArchDir,
> - StringRef Dir32,
> - StringRef Dir64,
> + /// Returns true if the \p Base path was found, false if it
[PATCH] D50943: Decl: allow "setInvalidDecl" for TagDecls
elsteveogrande created this revision.
elsteveogrande added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
For `TagDecl`s, complete or incomplete, allow `setInvalidDecl` calls.
Other `assert`s are interfering but we'll deal with them separately.
https://bugs.llvm.org/show_bug.cgi?id=38531
Repository:
rC Clang
https://reviews.llvm.org/D50943
Files:
lib/AST/DeclBase.cpp
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -131,11 +131,12 @@
void Decl::setInvalidDecl(bool Invalid) {
InvalidDecl = Invalid;
- assert(!isa(this) || !cast(this)->isCompleteDefinition());
if (!Invalid) {
return;
}
+ assert(!isa(this) || !cast(this)->isCompleteDefinition());
+
if (!isa(this)) {
// Defensive maneuver for ill-formed code: we're likely not to make it to
// a point where we set the access specifier, so default it to "public"
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -131,11 +131,12 @@
void Decl::setInvalidDecl(bool Invalid) {
InvalidDecl = Invalid;
- assert(!isa(this) || !cast(this)->isCompleteDefinition());
if (!Invalid) {
return;
}
+ assert(!isa(this) || !cast(this)->isCompleteDefinition());
+
if (!isa(this)) {
// Defensive maneuver for ill-formed code: we're likely not to make it to
// a point where we set the access specifier, so default it to "public"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50942: SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing
elsteveogrande created this revision.
elsteveogrande added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
Before checking that the overriding definition of a method is not more lax than
the overridden one, ensure the exception spec is parsed, and also evaluated.
Test case by Andrew Gallagher!
Repository:
rC Clang
https://reviews.llvm.org/D50942
Files:
lib/Sema/SemaExceptionSpec.cpp
test/Modules/Inputs/lax-base-except/a.h
test/Modules/Inputs/lax-base-except/module.modulemap
test/Modules/lax-base-except.cpp
Index: test/Modules/lax-base-except.cpp
===
--- /dev/null
+++ test/Modules/lax-base-except.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang -c -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/lax-base-except %s -o %t2.o
+// expected-no-diagnostics
+
+#include "a.h"
+
+class D : public A, public B {
+ public:
+ virtual ~D() override = default;
+};
Index: test/Modules/Inputs/lax-base-except/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/lax-base-except/module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "a.h"
+}
Index: test/Modules/Inputs/lax-base-except/a.h
===
--- /dev/null
+++ test/Modules/Inputs/lax-base-except/a.h
@@ -0,0 +1,19 @@
+class A {
+ public:
+ virtual ~A() = default;
+};
+
+class B {
+ public:
+ virtual ~B() {
+c.func();
+ }
+
+ struct C{
+void func() {}
+friend B::~B();
+ };
+
+ C c;
+};
+
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -899,11 +899,28 @@
bool Sema::CheckOverridingFunctionExceptionSpec(const CXXMethodDecl *New,
const CXXMethodDecl *Old) {
- // If the new exception specification hasn't been parsed yet, skip the check.
+ // If the new exception spec hasn't been determined yet, skip the check.
// We'll get called again once it's been parsed.
- if (New->getType()->castAs()->getExceptionSpecType() ==
- EST_Unparsed)
+ switch (New->getType()->castAs()->getExceptionSpecType()) {
+ case EST_Unparsed:
+ case EST_Unevaluated:
return false;
+ default:
+;
+ }
+
+ // If the old exception spec hasn't been determined yet, remember that
+ // we need to perform this check when we get to the end of the outermost
+ // lexically-surrounding class.
+ switch (New->getType()->castAs()->getExceptionSpecType()) {
+ case EST_Unparsed:
+ case EST_Unevaluated:
+DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
+return false;
+ default:
+;
+ }
+
if (getLangOpts().CPlusPlus11 && isa(New)) {
// Don't check uninstantiated template destructors at all. We can only
// synthesize correct specs after the template is instantiated.
@@ -916,17 +933,11 @@
return false;
}
}
- // If the old exception specification hasn't been parsed yet, remember that
- // we need to perform this check when we get to the end of the outermost
- // lexically-surrounding class.
- if (Old->getType()->castAs()->getExceptionSpecType() ==
- EST_Unparsed) {
-DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
-return false;
- }
+
unsigned DiagID = diag::err_override_exception_spec;
if (getLangOpts().MicrosoftExt)
DiagID = diag::ext_override_exception_spec;
+
return CheckExceptionSpecSubset(PDiag(DiagID),
PDiag(diag::err_deep_exception_specs_differ),
PDiag(diag::note_overridden_virtual_function),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41284: [Concepts] Associated constraints infrastructure.
Rakete added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); Quuxplusone wrote: > saar.raz wrote: > > Quuxplusone wrote: > > > For my own edification, could you explain whether, given > > > > > > #define BOOL bool > > > using typedef_for_bool = bool; > > > > > > you'd expect to diagnose a redeclaration of `B::A` with associated > > > constraint > > > > > > requires bool( U() ) // extra whitespace > > > > > > or > > > > > > requires BOOL(U()) // different spelling of `bool` > > > > > > or > > > > > > requires typedef_for_bool(U()) // different spelling of `bool` > > > > > > ? My naive reading of N4762 temp.constr.atomic/2 says that none of these > > > constraints (on line 10) would be "identical" to the constraint on line > > > 6... but then I don't understand what's the salient difference between > > > line 10 (which apparently gives no error) and line 22 (which apparently > > > gives an error). > > Line 22 has a not (!) operator in front of the bool(), I guess you missed > > that? > I saw the `!`... but I don't understand how the compiler "knows" that > `!bool(U())` is "different" from `bool(T())` in a way that doesn't equally > apply to `bool(U())`. > > Or suppose the constraint on line 10 was `requires bool(U())==true`... would > that give a diagnostic? `bool(T())` and `bool(U())` are identical because they have the same parameter mappings. The "identical" requirement applies to the actual grammar composition of the expression, so `bool(T())` would be different to `bool(T()) == true`. At least that's how I understand it. Repository: rC Clang https://reviews.llvm.org/D41284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41284: [Concepts] Associated constraints infrastructure.
Quuxplusone added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); saar.raz wrote: > Quuxplusone wrote: > > For my own edification, could you explain whether, given > > > > #define BOOL bool > > using typedef_for_bool = bool; > > > > you'd expect to diagnose a redeclaration of `B::A` with associated > > constraint > > > > requires bool( U() ) // extra whitespace > > > > or > > > > requires BOOL(U()) // different spelling of `bool` > > > > or > > > > requires typedef_for_bool(U()) // different spelling of `bool` > > > > ? My naive reading of N4762 temp.constr.atomic/2 says that none of these > > constraints (on line 10) would be "identical" to the constraint on line > > 6... but then I don't understand what's the salient difference between line > > 10 (which apparently gives no error) and line 22 (which apparently gives an > > error). > Line 22 has a not (!) operator in front of the bool(), I guess you missed > that? I saw the `!`... but I don't understand how the compiler "knows" that `!bool(U())` is "different" from `bool(T())` in a way that doesn't equally apply to `bool(U())`. Or suppose the constraint on line 10 was `requires bool(U())==true`... would that give a diagnostic? Repository: rC Clang https://reviews.llvm.org/D41284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50850: clang: Add triples support for MIPS r6
atanasyan added a comment. Could you add test cases to cover these changes? Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:115 + if (ABIName.empty() && (Triple.getEnvironment() == llvm::Triple::GNUABIN32)) +ABIName = "n32"; + It looks like this change is unrelated to introducing new target triples and can be made by a separate commit. Comment at: lib/Driver/ToolChains/Gnu.cpp:2068 BiarchTripleAliases.append(begin(MIPS64Triples), end(MIPS64Triples)); +BiarchLibDirs.append(begin(MIPSN32LibDirs), end(MIPSN32LibDirs)); +BiarchTripleAliases.append(begin(MIPSN32Triples), end(MIPSN32Triples)); Ditto Comment at: lib/Driver/ToolChains/Gnu.cpp:2077 BiarchTripleAliases.append(begin(MIPS64ELTriples), end(MIPS64ELTriples)); +BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs)); +BiarchTripleAliases.append(begin(MIPSN32ELTriples), end(MIPSN32ELTriples)); Ditto Comment at: lib/Driver/ToolChains/Gnu.cpp:2085 BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples)); +BiarchLibDirs.append(begin(MIPSN32LibDirs), end(MIPSN32LibDirs)); +BiarchTripleAliases.append(begin(MIPSN32Triples), end(MIPSN32Triples)); Ditto Comment at: lib/Driver/ToolChains/Gnu.cpp:2093 BiarchTripleAliases.append(begin(MIPSELTriples), end(MIPSELTriples)); -BiarchTripleAliases.append(begin(MIPSTriples), end(MIPSTriples)); +BiarchLibDirs.append(begin(MIPSN32ELLibDirs), end(MIPSN32ELLibDirs)); +BiarchTripleAliases.append(begin(MIPSN32ELTriples), end(MIPSN32ELTriples)); Ditto Comment at: lib/Driver/ToolChains/Gnu.cpp:2437 if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 || -getTriple().isAndroid() || -getTriple().isOSFreeBSD() || +getTriple().getEnvironment() == llvm::Triple::GNUABIN32 || +getTriple().isAndroid() || getTriple().isOSFreeBSD() || Are you sure that integrated LLVM assembler is ready to support N32 code generation? Anyway this change is for a separate patch. Comment at: lib/Driver/ToolChains/Linux.cpp:126 return "mips64-linux-gnu"; -if (D.getVFS().exists(SysRoot + "/lib/mips64-linux-gnuabi64")) - return "mips64-linux-gnuabi64"; +MipsCpu = (TargetSubArch == llvm::Triple::MipsSubArch_r6) ? "mipsisa64" + : "mips64"; Suppose there are both "/lib/mips64-linux-gnu" and "/lib/mipsisa64-linux-gnuabi64" paths on a disk and provided target triple is mipsisa64-linux-gnuabi64. Is it good that we return "mips64-linux-gnu" from this function anyway? Repository: rC Clang https://reviews.llvm.org/D50850 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50852: [clang-tidy] abseil-auto-make-unique
lebedev.ri added inline comments.
Comment at: clang-tidy/abseil/AutoMakeUniqueCheck.cpp:21
+void AutoMakeUniqueCheck::registerMatchers(MatchFinder* finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
zinovy.nis wrote:
> JonasToth wrote:
> > Please clang-format, `return` on next line.
> `auto` first appeared in C++11, so you may use `getLangOpts().CPlusPlus11`
> And `std::make_unique` is a part of C++14, so even `getLangOpts().CPlusPlus14.
I agree on the `auto` part, but it really shouldn't care any further than that.
You can write custom `yournamespace::MakeUnique<>` in C++11.
(As i have already wrote, the list if `MakeUnique` functions-to-be-searched
should be a config option.)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50852: [clang-tidy] abseil-auto-make-unique
zinovy.nis added inline comments.
Comment at: clang-tidy/abseil/AutoMakeUniqueCheck.cpp:21
+void AutoMakeUniqueCheck::registerMatchers(MatchFinder* finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
JonasToth wrote:
> Please clang-format, `return` on next line.
`auto` first appeared in C++11, so you may use `getLangOpts().CPlusPlus11`
And `std::make_unique` is a part of C++14, so even `getLangOpts().CPlusPlus14.
Comment at: clang-tidy/abseil/AutoMakeUniqueCheck.cpp:66
+ const auto* var_decl = result.Nodes.getNodeAs("var_decl");
+ const auto* make_unique_decl =
+ result.Nodes.getNodeAs("make_unique_decl");
You may move this definition after the next `if`+`return`.
Comment at: docs/clang-tidy/checks/abseil-auto-make-unique.rst:10
+
+ std::unique_ptr x = MakeUnique(...);
+
`MakeUnique`? Maybe `std/abseil::make_unique`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50852: [clang-tidy] abseil-auto-make-unique
zinovy.nis added a comment. In https://reviews.llvm.org/D50852#1202774, @lebedev.ri wrote: > 1. Please always upload all patches with full context. > 2. There already is `modernize-use-auto`. Does it handle this case? Then this > should be just an alias to that check. Else, i think it would be best to > extend that one, and still alias. First of all, thanks, Hugo, for your check! My +1 for extending `modernize-use-auto` as currently it doesn't support cases your check supports. Because replacing std::unique_ptr< int >y = std::make_unique(); with auto y = std::make_unique(); is logically the same as replacing long long int *y = new long long int (42); with auto* y = new long long int (42); So your check should extend the existing one and not create one more check IMHO. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50703: [clangd] NFC: Mark Workspace Symbol feature complete in the documentation
malaperle added a comment. In https://reviews.llvm.org/D50703#1205109, @kbobyrev wrote: > In https://reviews.llvm.org/D50703#1205049, @malaperle wrote: > > > I hadn't marked it as done because without symbols in main files I found it > > quite lacking. > > > Ah, I see, thank you for spotting it! I was under the impression that it's > feature-complete. Should I mark it "Partial" until this is fixed then? Nah, I guess other things are partial too, like Go to Definition. Let's assume they will be completed "soon" ;) Repository: rL LLVM https://reviews.llvm.org/D50703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug
elsteveogrande added a comment. hi @vsk + @arphaman , it's been a while -- I brought this old diff up to date, wondering if you could take a look again. Thanks! Repository: rC Clang https://reviews.llvm.org/D42043 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50862: [clang-tidy] Abseil: faster strsplit delimiter check
JonasToth added inline comments.
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:85
+
+ // absl::StrSplit(..., "x") -> absl::StrSplit(..., 'x')
+ // absl::ByAnyChar("x") -> 'x'
Maybe you could make these comments into full sentences. I do think its clear
what they mean, but the rule is full sentences.
Maybe `Find uses of 'absl::...' to transform them into 'absl::...'.`?
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:115
+
+ if (const auto *ByAnyChar = Result.Nodes.getNodeAs("ByAnyChar")) {
+Range = ByAnyChar->getSourceRange();
You can ellide the braces in this case.
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:49
+ // in the character literal.
+ if (Result == R"("'")") {
+return std::string(R"('\'')");
deannagarcia wrote:
> JonasToth wrote:
> > The comment suggest, that all single quotes need to be escaped and then
> > further processing happens, but you check on identity to `'` and return the
> > escaped version of it.
> > That confuses me.
> Does the new comment clear it up?
Yes, thank you.
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:54
+ // Now replace the " with '.
+ auto Pos = Result.find_first_of('"');
+ if (Pos == Result.npos)
deannagarcia wrote:
> JonasToth wrote:
> > This expects at max 2 `"` characters. couldnt there be more?
> The only way this will be run is if the string is a single character, so the
> only possibility if there are more than 2 " characters is that the character
> that is the delimiter is actually " which is taken care of in the check. Does
> that make sense/do you think I need to add a comment about this?
Ok I see, you could add an assertion before this section. Having the
precondition checked is always a good thing and usually adds clarity as well :)
Comment at: clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:75
+
+ // string_view passed by value and contructed from string literal.
+ auto StringViewArg =
deannagarcia wrote:
> JonasToth wrote:
> > What about the `std::string_view`?
> This matcher is only here for the next matcher to check to see if
> absl::ByAnyChar has been passed in a single character string view and is only
> necessary because ByAnyChar accepts an absl::string_view so it isn't
> necessary to make one for std::string_view
Ok.
Comment at: test/clang-tidy/abseil-faster-strsplit-delimiter.cpp:42
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning:
absl::StrSplit()/ByAnyChar()/MaxSplits() called with a string literal
consisting of a single character; consider using the more efficient overload
accepting a character [abseil-faster-strsplit-delimiter]
deannagarcia wrote:
> JonasToth wrote:
> > Please add a test, where `"A"` is used as an arbitray function argument
> > (similar to the template case, but without templates involved)
> Can you explain this more/provide an example?
The testcase i had in mind does not make sense, sorry for noise.
https://reviews.llvm.org/D50862
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41217: [Concepts] Concept Specialization Expressions
saar.raz updated this revision to Diff 161372.
saar.raz added a comment.
- Address CR comments - add FoundDecl tracking, instantiationdependent
calculation, display non-constant expression diagnostic notes
Repository:
rC Clang
https://reviews.llvm.org/D41217
Files:
include/clang/AST/DeclTemplate.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/StmtNodes.td
include/clang/Sema/Sema.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/Expr.cpp
lib/AST/ExprCXX.cpp
lib/AST/ExprClassification.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/StmtPrinter.cpp
lib/AST/StmtProfile.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Parse/ParseExpr.cpp
lib/Parse/ParseTemplate.cpp
lib/Sema/CMakeLists.txt
lib/Sema/SemaConcept.cpp
lib/Sema/SemaExceptionSpec.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInstantiate.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
test/Parser/cxx-concept-declaration.cpp
tools/libclang/CXCursor.cpp
Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -231,6 +231,7 @@
case Stmt::TypeTraitExprClass:
case Stmt::CoroutineBodyStmtClass:
case Stmt::CoawaitExprClass:
+ case Stmt::ConceptSpecializationExprClass:
case Stmt::DependentCoawaitExprClass:
case Stmt::CoreturnStmtClass:
case Stmt::CoyieldExprClass:
Index: test/Parser/cxx-concept-declaration.cpp
===
--- test/Parser/cxx-concept-declaration.cpp
+++ test/Parser/cxx-concept-declaration.cpp
@@ -9,8 +9,6 @@
template concept D1 = true; // expected-error {{expected template parameter}}
-template concept C2 = 0.f; // expected-error {{constraint expression must be 'bool'}}
-
struct S1 {
template concept C1 = true; // expected-error {{concept declarations may only appear in global or namespace scope}}
};
@@ -29,3 +27,31 @@
// TODO: Add test to prevent explicit specialization, partial specialization
// and explicit instantiation of concepts.
+
+template concept C7 = 2; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
+template concept C8 = 2 && x; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
+template concept C9 = x || 2 || x; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
+template concept C10 = 8ull && x || x; // expected-error {{atomic constraint must be of type 'bool' (found 'unsigned long long')}}
+template concept C11 = sizeof(T); // expected-error {{atomic constraint must be of type 'bool' (found 'unsigned long')}}
+template concept C12 = T{};
+static_assert(!C12);
+template concept C13 = (bool&&)true;
+static_assert(C13);
+template concept C14 = (const bool&)true;
+static_assert(C14);
+template concept C15 = (const bool)true;
+static_assert(C15);
+
+template
+struct integral_constant { static constexpr T value = v; };
+
+template concept C16 = integral_constant::value && true;
+static_assert(C16);
+static_assert(!C16);
+template concept C17 = integral_constant::value;
+static_assert(C17);
+static_assert(!C17);
+
+template concept C18 = integral_constant::value;
+// expected-error@-1{{use of undeclared identifier 'wor'; did you mean 'word'?}}
+// expected-note@-2{{'word' declared here}}
\ No newline at end of file
Index: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
===
--- test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
+++ test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
@@ -1,5 +1,151 @@
// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -verify %s
-// expected-no-diagnostics
-template concept C = true;
-static_assert(C);
+template concept C1 = true;
+static_assert(C1);
+
+template concept C2 = sizeof(T) == 4;
+static_assert(C2);
+static_assert(!C2);
+static_assert(C2);
+static_assert(!C2);
+
+template concept C3 = sizeof(*T{}) == 4;
+static_assert(C3);
+static_assert(!C3);
+
+struct A {
+ static constexpr int add(int a, int b) {
+return a + b;
+ }
+};
+struct B {
+ static int add(int a, int b) { // expected-note{{declared here}}
+return a + b;
+ }
+};
+template
+concept C4 = U::add(1, 2) == 3;
+// expected-error@-1{{substitution into constraint expression resulted in a non-constant expression}}
+// expected-note@-2{{non-constexpr function 'add' cannot be used in a constant expression}}
+static_assert(C4);
+static_assert(!C4); // expected-note {{while checking the satisfaction of concept 'C4' requested here}}
+
+template
+constexpr bool is_same_v = false;
+
+template
+constexpr bool is_same_v = true;
+
+template
+concept Same = is_same_v;
+
[PATCH] D50883: [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer
JonasToth added a comment. I see, thank you :) Am 17.08.2018 um 18:55 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D50883#1203690, @JonasToth wrote: > >> I am suprised that this does not automatically follow from the general >> rules. At the end, smartpointers cant do anything else then 'normal' classes. >> >> The `operator+/->` were not handled before? The mutation of `SmartPtr x; >> x->mf();` should already be catched, not? > > Different from `std::vector::operator[]` which has two overloads for const > and non-const access, `std::unique_ptr` only has one const version of > `operator->`. > So for `SmartPtr x; x->mf();` we only see a const operator being invoked on > `x`. `mf` is not a member of `SmartPtr` and the member call to `mf` is not on > `x` directly, we never followed that far. > > Repository: > > rCTE Clang Tools Extra > > https://reviews.llvm.org/D50883 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41284: [Concepts] Associated constraints infrastructure.
saar.raz added inline comments. Comment at: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp:10 + +template requires bool(U()) +int B::A = int(U()); Quuxplusone wrote: > For my own edification, could you explain whether, given > > #define BOOL bool > using typedef_for_bool = bool; > > you'd expect to diagnose a redeclaration of `B::A` with associated constraint > > requires bool( U() ) // extra whitespace > > or > > requires BOOL(U()) // different spelling of `bool` > > or > > requires typedef_for_bool(U()) // different spelling of `bool` > > ? My naive reading of N4762 temp.constr.atomic/2 says that none of these > constraints (on line 10) would be "identical" to the constraint on line 6... > but then I don't understand what's the salient difference between line 10 > (which apparently gives no error) and line 22 (which apparently gives an > error). Line 22 has a not (!) operator in front of the bool(), I guess you missed that? Repository: rC Clang https://reviews.llvm.org/D41284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41217: [Concepts] Concept Specialization Expressions
saar.raz added inline comments. Comment at: lib/Sema/SemaTemplateInstantiate.cpp:679-681 + Diags.Report(Active->PointOfInstantiation, + diag::note_constraint_substitution_here) + << Active->InstantiationRange; saar.raz wrote: > rsmith wrote: > > Is this note ever useful? It will presumably always point into the same > > concept definition that the prior diagnostic also pointed at, and doesn't > > seem to add anything in the testcases. > > > > Maybe we could keep the CodeSynthesisContext around as a marker that we've > > entered a SFINAE context, but not have any corresponding diagnostic. (The > > note produced for the enclosing `ConstraintsCheck` context covers that.) Or > > we could remove this and store a flag on the `ConstraintsCheck` to indicate > > whether we're in a SFINAEable portion of it. > If the concept definition is multiline/contains macros, this would point at > the exact place where the problematic constraint occured, we should probably > add tests for this case though. > Maybe we can omit the diagnostic when the concept and the constraint are on > the same line or something? > Correction - I just now realized you were referring to the 'in instantiation of template class' note and not the 'while checking the satisfaction...' note. In this case - the only case I can think of where the problematic instantiation and the constraint expression would be in very different places is indeed multiline constraint expressions or macros in a constraint expression (but you know better than I do - can you think of any other cases? or maybe there are other non-SFINAE errors that can result from substitution and would not produce a note?). Maybe these cases are remote enough to warrant removing this diagnostic or as I said earlier omit them in cases where they are unhelpful. Repository: rC Clang https://reviews.llvm.org/D41217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41217: [Concepts] Concept Specialization Expressions
saar.raz added inline comments. Comment at: lib/Sema/SemaTemplateInstantiate.cpp:679-681 + Diags.Report(Active->PointOfInstantiation, + diag::note_constraint_substitution_here) + << Active->InstantiationRange; rsmith wrote: > Is this note ever useful? It will presumably always point into the same > concept definition that the prior diagnostic also pointed at, and doesn't > seem to add anything in the testcases. > > Maybe we could keep the CodeSynthesisContext around as a marker that we've > entered a SFINAE context, but not have any corresponding diagnostic. (The > note produced for the enclosing `ConstraintsCheck` context covers that.) Or > we could remove this and store a flag on the `ConstraintsCheck` to indicate > whether we're in a SFINAEable portion of it. If the concept definition is multiline/contains macros, this would point at the exact place where the problematic constraint occured, we should probably add tests for this case though. Maybe we can omit the diagnostic when the concept and the constraint are on the same line or something? Repository: rC Clang https://reviews.llvm.org/D41217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50703: [clangd] NFC: Mark Workspace Symbol feature complete in the documentation
kbobyrev added a comment. In https://reviews.llvm.org/D50703#1205049, @malaperle wrote: > I hadn't marked it as done because without symbols in main files I found it > quite lacking. Ah, I see, thank you for spotting it! I was under the impression that it's feature-complete. Should I mark it "Partial" until this is fixed then? Repository: rL LLVM https://reviews.llvm.org/D50703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
