[PATCH] D40508: Replace long type names in IR with hashes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Are you trying to use LLVM struct type identity to infer information about the 
source program?  That is not and has never been a guarantee.


https://reviews.llvm.org/D40508



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

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D40463#939772, @dcoughlin wrote:

> Thanks, this looks good to me!


Awesome, thank you!

> Do you have commit access or do you need someone to commit it for you?

I do have commit access.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

[PATCH] D40508: Replace long type names in IR with hashes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In https://reviews.llvm.org/D40508#939513, @rjmccall wrote:

> In https://reviews.llvm.org/D40508#938854, @sepavloff wrote:
>
> > In https://reviews.llvm.org/D40508#938686, @rjmccall wrote:
> >
> > > The LLVM linking model does not actually depend on struct type names 
> > > matching.  My understanding is that, at best, it uses that as a heuristic 
> > > for deciding whether to make an effort to unify two types, but it's not 
> > > something that's ultimately supposed to impact IR semantics.
> >
> >
> > It is mainly true with an exception, when `llvm-link` resolves opaque types 
> > it relies on type name only. And this behavior creates the issue that 
> > https://reviews.llvm.org/D40567 tries to resolve.
>
>
> It is not clear from that report what the actual problem is.  Two incomplete 
> types get merged by the linker?  So what?


`llvm-link` is expected to produce IR that is semantically consistent with the 
program initially represented by a set of TUs. In this case it is not true. A 
function defined in source as `foo(ABC&)` is converted by linking to 
`foo &)` and this breaks initial semantics.

>>> If we needed IR type names to match reliably, we would use a mangled name, 
>>> not a pretty-print.
>> 
>> There is no requirement for IR type name to be an identifier, so 
>> pretty-print fits the need of type identification.
> 
> Not really; pretty-printing drops a lot of information that is pertinent in a 
> stable identifier, like scopes and so on, and makes arbitrary decisions about 
> other things, like where to insert spaces, namespace qualifiers, etc.

Type name mangling indeed is attractive solution. It has at least the 
advantages:

- It is reversible (in theory).
- It can be more compact. For instance, there is no need to spell type name 
that is encountered already, a some kind of reference is sufficient.

And there are arguments against it:

- It make working with IR harder for developers as readability is broken,
- Type name in IR is mostly a decoration, with the exception of rare case of 
opaque type resolution, so type name mangling may be considered as an overkill.

On the other hand pretty-printing can be finely tuned so that all necessary 
information appears in its result. As there is no requirements on compatibility 
of type names in bitcode files, things like number of spaces look not so 
important, it is enough that the same version of clang was used to compile bc 
files that are linked by `llvm-link`. After all it is readable.


https://reviews.llvm.org/D40508



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

[PATCH] D40637: [CMake] Support runtimes and monorepo layouts when looking for libc++

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
Herald added subscribers: Sanitizers, llvm-commits, mgorny, kubamracek.
Herald added a reviewer: EricWF.

This also slightly refactors the code that's checking the directory
presence which allows eliminating one unnecessary variable.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40637

Files:
  CMakeLists.txt
  cmake/Modules/AddCompilerRT.cmake
  lib/msan/tests/CMakeLists.txt
  lib/tsan/CMakeLists.txt
  test/msan/CMakeLists.txt
  test/tsan/CMakeLists.txt


Index: test/tsan/CMakeLists.txt
===
--- test/tsan/CMakeLists.txt
+++ test/tsan/CMakeLists.txt
@@ -7,7 +7,7 @@
 if(NOT COMPILER_RT_STANDALONE_BUILD)
   list(APPEND TSAN_TEST_DEPS tsan)
 endif()
-if(COMPILER_RT_HAS_LIBCXX_SOURCES AND
+if(COMPILER_RT_LIBCXX_PATH AND
COMPILER_RT_TEST_COMPILER_ID STREQUAL "Clang"
AND NOT APPLE AND NOT ANDROID)
   list(APPEND TSAN_TEST_DEPS libcxx_tsan)
Index: test/msan/CMakeLists.txt
===
--- test/msan/CMakeLists.txt
+++ test/msan/CMakeLists.txt
@@ -25,7 +25,7 @@
   list(APPEND MSAN_TEST_DEPS msan)
 endif()
 
-if(COMPILER_RT_INCLUDE_TESTS AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+if(COMPILER_RT_INCLUDE_TESTS AND COMPILER_RT_LIBCXX_PATH)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
 ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg)
Index: lib/tsan/CMakeLists.txt
===
--- lib/tsan/CMakeLists.txt
+++ lib/tsan/CMakeLists.txt
@@ -223,7 +223,7 @@
 endif()
 
 # Build libcxx instrumented with TSan.
-if(COMPILER_RT_HAS_LIBCXX_SOURCES AND
+if(COMPILER_RT_LIBCXX_PATH AND
COMPILER_RT_TEST_COMPILER_ID STREQUAL "Clang" AND
NOT ANDROID)
   set(libcxx_tsan_deps)
Index: lib/msan/tests/CMakeLists.txt
===
--- lib/msan/tests/CMakeLists.txt
+++ lib/msan/tests/CMakeLists.txt
@@ -122,7 +122,7 @@
 endmacro()
 
 # We should only build MSan unit tests if we can build instrumented libcxx.
-if(COMPILER_RT_CAN_EXECUTE_TESTS AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+if(COMPILER_RT_CAN_EXECUTE_TESTS AND COMPILER_RT_LIBCXX_PATH)
   foreach(arch ${MSAN_SUPPORTED_ARCH})
 get_target_flags_for_arch(${arch} TARGET_CFLAGS)
 set(LIBCXX_PREFIX ${CMAKE_CURRENT_BINARY_DIR}/../libcxx_msan_${arch})
Index: cmake/Modules/AddCompilerRT.cmake
===
--- cmake/Modules/AddCompilerRT.cmake
+++ cmake/Modules/AddCompilerRT.cmake
@@ -438,7 +438,7 @@
 #   DEPS 
 #   CFLAGS )
 macro(add_custom_libcxx name prefix)
-  if(NOT COMPILER_RT_HAS_LIBCXX_SOURCES)
+  if(NOT COMPILER_RT_LIBCXX_PATH)
 message(FATAL_ERROR "libcxx not found!")
   endif()
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -348,17 +348,15 @@
 
 add_subdirectory(include)
 
-set(COMPILER_RT_LIBCXX_PATH ${LLVM_MAIN_SRC_DIR}/projects/libcxx)
-if(EXISTS ${COMPILER_RT_LIBCXX_PATH}/)
-  set(COMPILER_RT_HAS_LIBCXX_SOURCES TRUE)
-else()
-  set(COMPILER_RT_LIBCXX_PATH ${LLVM_MAIN_SRC_DIR}/../libcxx)
-  if(EXISTS ${COMPILER_RT_LIBCXX_PATH}/)
-set(COMPILER_RT_HAS_LIBCXX_SOURCES TRUE)
-  else()
-set(COMPILER_RT_HAS_LIBCXX_SOURCES FALSE)
+foreach(path IN ITEMS ${LLVM_MAIN_SRC_DIR}/projects/libcxx
+  ${LLVM_MAIN_SRC_DIR}/runtimes/libcxx
+  ${LLVM_MAIN_SRC_DIR}/../libcxx
+  ${LLVM_EXTERNAL_LIBCXX_SOURCE_DIR})
+  if(IS_DIRECTORY ${path})
+set(COMPILER_RT_LIBCXX_PATH ${path})
+break()
   endif()
-endif()
+endforeach()
 
 set(COMPILER_RT_LLD_PATH ${LLVM_MAIN_SRC_DIR}/tools/lld)
 if(EXISTS ${COMPILER_RT_LLD_PATH}/ AND LLVM_TOOL_LLD_BUILD)


Index: test/tsan/CMakeLists.txt
===
--- test/tsan/CMakeLists.txt
+++ test/tsan/CMakeLists.txt
@@ -7,7 +7,7 @@
 if(NOT COMPILER_RT_STANDALONE_BUILD)
   list(APPEND TSAN_TEST_DEPS tsan)
 endif()
-if(COMPILER_RT_HAS_LIBCXX_SOURCES AND
+if(COMPILER_RT_LIBCXX_PATH AND
COMPILER_RT_TEST_COMPILER_ID STREQUAL "Clang"
AND NOT APPLE AND NOT ANDROID)
   list(APPEND TSAN_TEST_DEPS libcxx_tsan)
Index: test/msan/CMakeLists.txt
===
--- test/msan/CMakeLists.txt
+++ test/msan/CMakeLists.txt
@@ -25,7 +25,7 @@
   list(APPEND MSAN_TEST_DEPS msan)
 endif()
 
-if(COMPILER_RT_INCLUDE_TESTS AND COMPILER_RT_HAS_LIBCXX_SOURCES)
+if(COMPILER_RT_INCLUDE_TESTS AND COMPILER_RT_LIBCXX_PATH)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/Unit/lit.site.cfg.in
 ${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg)
Index: lib/tsan/CMakeLists.txt
===
--- lib/tsan/CMakeLists.txt
+++ lib/tsan/CMakeLists.txt
@@ -223,7 +223,7 @@
 

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1117
+
+  if (!DeclVector.empty()) {
+const Decl *LocationDecl = DeclVector[0];

malaperle wrote:
> malaperle wrote:
> > malaperle wrote:
> > > I think we need to simplify this part a bit. I'll try to find a way. Feel 
> > > free to wait until more comments before updating.
> > Here's the version in which I tried to simplify this *a bit*. With the new 
> > ErrorOr checks as well.
> > 
> > ```
> > Hover clangd::getHover(ParsedAST , Position Pos, clangd::Logger 
> > ) {
> >   const SourceManager  = AST.getASTContext().getSourceManager();
> >   const LangOptions  = AST.getASTContext().getLangOpts();
> >   const FileEntry *FE = 
> > SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
> >   if (!FE)
> > return Hover();
> > 
> >   SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
> > 
> >   auto DeclMacrosFinder = std::make_shared(
> >   llvm::errs(), SourceLocationBeg, AST.getASTContext(),
> >   AST.getPreprocessor());
> >   index::IndexingOptions IndexOpts;
> >   IndexOpts.SystemSymbolFilter =
> >   index::IndexingOptions::SystemSymbolFilterKind::All;
> >   IndexOpts.IndexFunctionLocals = true;
> >   indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
> >  DeclMacrosFinder, IndexOpts);
> > 
> >   auto Decls = DeclMacrosFinder->takeDecls();
> >   if (!Decls.empty() && Decls[0]) {
> > const Decl *LocationDecl = Decls[0];
> > std::vector HoverContents;
> > 
> > // Compute scope as the first "section" of the hover.
> > if (const NamedDecl *NamedD = dyn_cast(LocationDecl)) {
> >   std::string Scope = getScope(NamedD, 
> > AST.getASTContext().getLangOpts());
> >   if (!Scope.empty()) {
> > MarkedString Info = {"", "C++", "In " + Scope};
> > HoverContents.push_back(Info);
> >   }
> > }
> > 
> > // Adjust beginning of hover text depending on the presence of 
> > templates and comments.
> > TemplateDecl * TDec = LocationDecl->getDescribedTemplate();
> > const Decl * BeginDecl = TDec ? TDec : LocationDecl;
> > SourceLocation BeginLocation = BeginDecl->getSourceRange().getBegin();
> > RawComment *RC = AST.getASTContext().getRawCommentForDeclNoCache(
> > BeginDecl);
> > if (RC)
> >   BeginLocation = RC->getLocStart();
> > 
> > // Adjust end of hover text for things that have braces/bodies. We don't
> > // want to show the whole definition of a function, class, etc.
> > SourceLocation EndLocation = LocationDecl->getSourceRange().getEnd();
> > if (auto FuncDecl = dyn_cast(LocationDecl)) {
> >   if (FuncDecl->isThisDeclarationADefinition() && FuncDecl->getBody())
> > EndLocation = FuncDecl->getBody()->getLocStart();
> > } else if (auto TagDeclaration = dyn_cast(LocationDecl)) {
> >   if (TagDeclaration->isThisDeclarationADefinition())
> > EndLocation = TagDeclaration->getBraceRange().getBegin();
> > } else if (auto NameDecl = dyn_cast(LocationDecl)) {
> >   SourceLocation BeforeLBraceLoc = Lexer::getLocForEndOfToken(
> >   LocationDecl->getLocation(), 0, SourceMgr, LangOpts);
> >   if (BeforeLBraceLoc.isValid())
> > EndLocation = BeforeLBraceLoc;
> > }
> > 
> > SourceRange SR(BeginLocation, EndLocation);
> > if (SR.isValid()) {
> >   auto L = getDeclarationLocation(AST, SR);
> >   if (L) {
> > auto Ref = getBufferDataFromSourceRange(AST, *L);
> > if (Ref) {
> >   Hover H;
> >   if (SourceMgr.isInMainFile(BeginLocation))
> > H.range = L->range;
> >   MarkedString MS = {"", "C++", *Ref};
> >   HoverContents.push_back(MS);
> >   H.contents = std::move(HoverContents);
> >   return H;
> > }
> >   }
> > }
> >   }
> > 
> >   auto MacroInfos = DeclMacrosFinder->takeMacroInfos();
> >   if (!MacroInfos.empty() && MacroInfos[0]) {
> > const MacroInfo *MacroInf = MacroInfos[0];
> > SourceRange SR(MacroInf->getDefinitionLoc(),
> >  MacroInf->getDefinitionEndLoc());
> > if (SR.isValid()) {
> >   auto L = getDeclarationLocation(AST, SR);
> >   if (L) {
> > auto Ref = getBufferDataFromSourceRange(AST, *L);
> > if (Ref) {
> >   Hover H;
> >   if (SourceMgr.isInMainFile(SR.getBegin()))
> > H.range = L->range;
> >   MarkedString MS = {"", "C++", "#define " + Ref->str()};
> >   H.contents.push_back(MS);
> >   return H;
> > }
> >   }
> > }
> >   }
> > 
> >   return Hover();
> > }
> > ```
> Here's the version in which I tried to simplify this *a bit*. With the new 
> ErrorOr checks as well.
> 
> ```
> Hover clangd::getHover(ParsedAST , Position Pos, clangd::Logger ) {
>   const SourceManager  = AST.getASTContext().getSourceManager();
>   const LangOptions  = 

[PATCH] D40567: Always show template parameters in IR type names

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In https://reviews.llvm.org/D40567#940025, @rsmith wrote:

> What actual problem is this solving? These IR type names exist only for the 
> convenience of humans reading the IR, and making them long (potentially 
> extremely long) by including template arguments seems likely to hinder that 
> more than it helps.


There is a case when IR type names are essential. If `llvm-link` sees opaque 
type in one TU, it tries to merge it with its definition in another TU. The 
type name is used to identify proper definition. All template specializations 
share the same name, so `llvm-link` has to choose random type, which results in 
incorrect IR. Such IR prevents from some whole-program analyses. This problem 
cannot be solved by `llvm-link`, as some information is already lost.

Possible IR bloating due to long type names is indeed an issue. 
https://reviews.llvm.org/D40508 tries to solve it by replacing long type name 
or part of it by SHA1 hash. Type names become shorter and still are usable for 
type identification across different TUs. Template arguments are added to the 
type names only if compilation produces IR (including embedded IR in LTO), 
machine code generation is not affected.

If adding template arguments in all cases is not appropriate, probably it makes 
sense to have an option, something like '--precise-ir', that would turn this 
generation on? It could be used in the cases when accurate type information in 
IR is required.


https://reviews.llvm.org/D40567



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

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Peter Szecsi via Phabricator via cfe-commits]

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



Comment at: unittests/AST/ASTImporterTest.cpp:611
+  EXPECT_TRUE(testImport("template  struct S;"
+ "template  void declToImport() {"
+ "  S::foo;"

a.sidorin wrote:
> Uninstantiated templates make me worry about Windows buildbots. If they will 
> start to fail, we should test these matchers with `-fms-compatibility` and 
> `-fdelayed-template-parsing` options enabled.
Not sure about Windows but I can ensure that this test has passed with these 
args added as well.


https://reviews.llvm.org/D38845



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

[PATCH] D40625: Harmonizing attribute GNU/C++ spellings

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: include/clang/Basic/Attr.td:602
 def AnalyzerNoReturn : InheritableAttr {
-  let Spellings = [GNU<"analyzer_noreturn">];
+  let Spellings = [Clang<"analyzer_noreturn">];
   let Documentation = [Undocumented];

Hmm, should the clang static analyzer reuse the `clang::` namespace, or should 
it get its own?



Comment at: include/clang/Basic/Attr.td:649
 def Availability : InheritableAttr {
-  let Spellings = [GNU<"availability">];
+  let Spellings = [Clang<"availability">];
   let Args = [IdentifierArgument<"platform">, VersionArgument<"introduced">,

Does the custom parsing for this work for the C++11 attribute syntax?



Comment at: include/clang/Basic/Attr.td:1218-1228
 def NeonPolyVectorType : TypeAttr {
-  let Spellings = [GNU<"neon_polyvector_type">];
+  let Spellings = [Clang<"neon_polyvector_type">];
   let Args = [IntArgument<"NumElements">];
   let Documentation = [Undocumented];
 }
 
 def NeonVectorType : TypeAttr {

I *think* these are a Clang invention rather than part of the ARM NEON 
intrinsics specification, but perhaps you could ask someone from ARM to confirm 
that.


https://reviews.llvm.org/D40625



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D39347#940015, @rsmith wrote:

> Thanks for your patience, this turned out to be surprisingly complicated.


Of course!  Thank you as well for your input, I was surprised at how 
complicated this ended up being as well!


https://reviews.llvm.org/D39347



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

[PATCH] D40567: Always show template parameters in IR type names

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

What actual problem is this solving? These IR type names exist only for the 
convenience of humans reading the IR, and making them long (potentially 
extremely long) by including template arguments seems likely to hinder that 
more than it helps.


https://reviews.llvm.org/D40567



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks for your patience, this turned out to be surprisingly complicated.


https://reviews.llvm.org/D39347



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 124863.
erichkeane added a comment.

Fix for trailing padding BITS, which obviously won't be merged.


https://reviews.llvm.org/D39347

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/CXXABI.h
  lib/AST/ItaniumCXXABI.cpp
  lib/AST/MicrosoftCXXABI.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/has_unique_object_reps_member_ptr.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
   return false;
 }
 
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isUnionType() && "must be union type");
-  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
-  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
-  for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
-  return false;
-  }
-  return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
-  assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
-  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
-  if (!RD->field_empty())
-return false;
-
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
-  }
-
-  return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isStructureOrClassType() && "Must be struct or class");
-  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
-  if (isStructEmpty(*this))
-return false;
-
-  // Check base types.
-  CharUnits BaseSize{};
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
-  if (Base.isVirtual())
-return false;
-
-  // Empty bases are permitted, otherwise ensure base has unique
-  // representation. Also, Empty Base Optimization means that an
-  // Empty base takes up 0 size.
-  if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
-  return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
-  }
-}
-  }
-
-  CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
-  // This struct obviously has bases that keep it from being 'empty', so
-  // checking fields is no longer required.  Ensure that the struct size
-  // is the sum of the bases.
-  if (RD->field_empty())
-return StructSize == BaseSize;
-
-  CharUnits CurOffset =
-  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
-  // If the first field isn't at the sum of the size of the bases, there
-  // is padding somewhere.
-  if (BaseSize != CurOffset)
-return false;
-
-  for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
-  return false;
-CurOffset += FieldSize;
-  }
-  // Check for tail padding.
-  return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext ) const {
-  // C++17 [meta.unary.prop]:
-  //   The predicate condition for a template specialization
-  //   has_unique_object_representations shall be
-  //   satisfied if and only if:
-  // (9.1) - T is trivially copyable, and
-  // (9.2) - any two objects of type T with the same value have the same
-  // object representation, where two objects
-  //   of array or non-union class type are considered to have the same value
-  //   if their respective sequences of
-  //   direct subobjects have the same values, and two objects of union type
-  //   are considered to have the same
-  //   value if they have the same active member and the corresponding members
-  //   have the same value.
-  //   The set of scalar types for which this condition holds is
-  //   implementation-defined. [ Note: If a type has padding
-  //   bits, the condition does not hold; otherwise, the condition holds true
-  //   for unsigned integral types. -- end note ]
-  if (isNull())
-return false;
-
-  // Arrays are unique only if their element type is unique.
-  if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
-  // (9.1) - T is trivially copyable, and
-  if (!isTriviallyCopyableType(Context))
-return false;
-
-  // Functions are not unique.
-  if ((*this)->isFunctionType())
-return false;
-
- 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/ASTContext.cpp:2200
+  Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+  CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+  if (BaseOffset != CurOffset)

rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > erichkeane wrote:
> > > > rsmith wrote:
> > > > > On reflection, I don't think this is right, and likewise I don't 
> > > > > think the check for unique object representations of the base class 
> > > > > above is quite right.
> > > > > 
> > > > > A class can be standard-layout but not POD for the purposes of layout 
> > > > > (eg, by declaring a special member function). If so, the derived 
> > > > > class can reuse the base class's tail padding, and if it does, the 
> > > > > derived class can have unique object representations even when the 
> > > > > base class does not. Example:
> > > > > 
> > > > > ```
> > > > > struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
> > > > > ```
> > > > > 
> > > > > Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique 
> > > > > object representations even though its base class `A` does not.
> > > > > 
> > > > > 
> > > > > This should be fairly easy to fix. One approach would be to change 
> > > > > the recursive call to `hasUniqueObjectRepresentations` above to 
> > > > > return the actual size occupied by the struct and its fields (rather 
> > > > > than checking for tail padding), and add that size here instead of 
> > > > > using the base size. Or you could query the DataSize in the record 
> > > > > layout rather than getting the (complete object) type size (but you'd 
> > > > > then still need to check that there's no bits of tail padding before 
> > > > > the end of the dsize, since we still pad out trailing bit-fields in 
> > > > > the dsize computation).
> > > > According to the standard, the above case isn't, because it is 
> > > > non-trivial, right?  9.1 requires that "T" (B in your case) be 
> > > > trivially copyable, which it isn't, right?
> > > > 
> > > > The predicate condition for a template specialization
> > > > has_unique_object_representations shall be
> > > > satisfied if and only if:
> > > > (9.1) - T is trivially copyable, and
> > > > (9.2) - any two objects of type T with the same value have the same
> > > > object representation
> > > Fixed example:
> > > 
> > > ```
> > > struct A { int  char a; }; struct B : A { char b[7]; };
> > > ```
> > AH!  I got caught up by the destructor as the reason it wasn't unique, but 
> > the actual thing you meant is that "A" has tail padding, so it is NOT 
> > unique.  However, on Itanium, the tail padding gets used when inheriting 
> > from it.
> > 
> > Do I have that correct? I just have to fix the behavior of 
> > inheriting-removing-tail-padding?
> You have fallen into a trap :) There can be (up to 7 bits of) padding between 
> the end of the members and the end of the data size, specifically if the 
> struct ends in a bit-field. You could check for that before you return 
> `CurOffsetInBits` at the end of this function, but I think it'd be better to 
> store the size in `Bases` and just use that down here rather than grabbing 
> and using the data size.
Gah!  You're right!  Fix incoming.


https://reviews.llvm.org/D39347



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/AST/ASTContext.cpp:2200
+  Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+  CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+  if (BaseOffset != CurOffset)

erichkeane wrote:
> rsmith wrote:
> > erichkeane wrote:
> > > rsmith wrote:
> > > > On reflection, I don't think this is right, and likewise I don't think 
> > > > the check for unique object representations of the base class above is 
> > > > quite right.
> > > > 
> > > > A class can be standard-layout but not POD for the purposes of layout 
> > > > (eg, by declaring a special member function). If so, the derived class 
> > > > can reuse the base class's tail padding, and if it does, the derived 
> > > > class can have unique object representations even when the base class 
> > > > does not. Example:
> > > > 
> > > > ```
> > > > struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
> > > > ```
> > > > 
> > > > Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique 
> > > > object representations even though its base class `A` does not.
> > > > 
> > > > 
> > > > This should be fairly easy to fix. One approach would be to change the 
> > > > recursive call to `hasUniqueObjectRepresentations` above to return the 
> > > > actual size occupied by the struct and its fields (rather than checking 
> > > > for tail padding), and add that size here instead of using the base 
> > > > size. Or you could query the DataSize in the record layout rather than 
> > > > getting the (complete object) type size (but you'd then still need to 
> > > > check that there's no bits of tail padding before the end of the dsize, 
> > > > since we still pad out trailing bit-fields in the dsize computation).
> > > According to the standard, the above case isn't, because it is 
> > > non-trivial, right?  9.1 requires that "T" (B in your case) be trivially 
> > > copyable, which it isn't, right?
> > > 
> > > The predicate condition for a template specialization
> > > has_unique_object_representations shall be
> > > satisfied if and only if:
> > > (9.1) - T is trivially copyable, and
> > > (9.2) - any two objects of type T with the same value have the same
> > > object representation
> > Fixed example:
> > 
> > ```
> > struct A { int  char a; }; struct B : A { char b[7]; };
> > ```
> AH!  I got caught up by the destructor as the reason it wasn't unique, but 
> the actual thing you meant is that "A" has tail padding, so it is NOT unique. 
>  However, on Itanium, the tail padding gets used when inheriting from it.
> 
> Do I have that correct? I just have to fix the behavior of 
> inheriting-removing-tail-padding?
You have fallen into a trap :) There can be (up to 7 bits of) padding between 
the end of the members and the end of the data size, specifically if the struct 
ends in a bit-field. You could check for that before you return 
`CurOffsetInBits` at the end of this function, but I think it'd be better to 
store the size in `Bases` and just use that down here rather than grabbing and 
using the data size.


https://reviews.llvm.org/D39347



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

juliehockett wrote:
> alexfh wrote:
> > This is not about the check, rather about the underlying style guide. The 
> > document linked here doesn't explain why certain features are disallowed. 
> > I'd suggest putting some effort in expanding the document to include 
> > reasoning for each rule (e.g. see 
> > https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance for 
> > a related rule in the Google C++ style guide).
> Good point -- we're looking into updating it. Thanks!
Hmm, the document linked here explicitly says that multiple inheritance of 
non-interface classes is allowed:

> * Allowed
>* ...
>* Multiple implementation inheritance
>* But be judicious. This is used widely for e.g. intrusive container 
> mixins.

That seems to directly contradict the warning produced by this check, 
`inheriting mulitple classes which aren't pure virtual is disallowed`. Should 
it just say "... is discouraged"?


https://reviews.llvm.org/D40580



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 124859.
erichkeane added a comment.

Fixed the 'tail padding' mechanism to work correctly as suggested by @rsmith


https://reviews.llvm.org/D39347

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  lib/AST/ASTContext.cpp
  lib/AST/CXXABI.h
  lib/AST/ItaniumCXXABI.cpp
  lib/AST/MicrosoftCXXABI.cpp
  lib/AST/Type.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/has_unique_object_reps_member_ptr.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2201,150 +2201,6 @@
   return false;
 }
 
-bool QualType::unionHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isUnionType() && "must be union type");
-  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
-  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
-
-  for (const auto *Field : Union->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-if (FieldSize != UnionSize)
-  return false;
-  }
-  return true;
-}
-
-static bool isStructEmpty(QualType Ty) {
-  assert(Ty.getTypePtr()->isStructureOrClassType() &&
- "Must be struct or class");
-  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
-
-  if (!RD->field_empty())
-return false;
-
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-return ClassDecl->isEmpty();
-  }
-
-  return true;
-}
-
-bool QualType::structHasUniqueObjectRepresentations(
-const ASTContext ) const {
-  assert((*this)->isStructureOrClassType() && "Must be struct or class");
-  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
-
-  if (isStructEmpty(*this))
-return false;
-
-  // Check base types.
-  CharUnits BaseSize{};
-  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
-for (const auto Base : ClassDecl->bases()) {
-  if (Base.isVirtual())
-return false;
-
-  // Empty bases are permitted, otherwise ensure base has unique
-  // representation. Also, Empty Base Optimization means that an
-  // Empty base takes up 0 size.
-  if (!isStructEmpty(Base.getType())) {
-if (!Base.getType().structHasUniqueObjectRepresentations(Context))
-  return false;
-BaseSize += Context.getTypeSizeInChars(Base.getType());
-  }
-}
-  }
-
-  CharUnits StructSize = Context.getTypeSizeInChars(*this);
-
-  // This struct obviously has bases that keep it from being 'empty', so
-  // checking fields is no longer required.  Ensure that the struct size
-  // is the sum of the bases.
-  if (RD->field_empty())
-return StructSize == BaseSize;
-
-  CharUnits CurOffset =
-  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
-
-  // If the first field isn't at the sum of the size of the bases, there
-  // is padding somewhere.
-  if (BaseSize != CurOffset)
-return false;
-
-  for (const auto *Field : RD->fields()) {
-if (!Field->getType().hasUniqueObjectRepresentations(Context))
-  return false;
-CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
-CharUnits FieldOffset =
-Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
-// Has padding between fields.
-if (FieldOffset != CurOffset)
-  return false;
-CurOffset += FieldSize;
-  }
-  // Check for tail padding.
-  return CurOffset == StructSize;
-}
-
-bool QualType::hasUniqueObjectRepresentations(const ASTContext ) const {
-  // C++17 [meta.unary.prop]:
-  //   The predicate condition for a template specialization
-  //   has_unique_object_representations shall be
-  //   satisfied if and only if:
-  // (9.1) - T is trivially copyable, and
-  // (9.2) - any two objects of type T with the same value have the same
-  // object representation, where two objects
-  //   of array or non-union class type are considered to have the same value
-  //   if their respective sequences of
-  //   direct subobjects have the same values, and two objects of union type
-  //   are considered to have the same
-  //   value if they have the same active member and the corresponding members
-  //   have the same value.
-  //   The set of scalar types for which this condition holds is
-  //   implementation-defined. [ Note: If a type has padding
-  //   bits, the condition does not hold; otherwise, the condition holds true
-  //   for unsigned integral types. -- end note ]
-  if (isNull())
-return false;
-
-  // Arrays are unique only if their element type is unique.
-  if ((*this)->isArrayType())
-return Context.getBaseElementType(*this).hasUniqueObjectRepresentations(
-Context);
-
-  // (9.1) - T is trivially copyable, and
-  if (!isTriviallyCopyableType(Context))
-return false;
-
-  // Functions are not unique.
-  if ((*this)->isFunctionType())
-

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

If nothing uses `getCXXScopeSpecifier` right now we can't really test it with a 
clang or c-index-test regression test. A completion unit test could work here. 
I don't think we actually have existing completion unit tests though, so you 
would have to create one from scratch. But if `getCXXScopeSpecifier` will be 
used in a follow up patch maybe it will be easier to commit this without a test 
together with the followup patch?


https://reviews.llvm.org/D40563



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

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Here's a simple test that breaks:

  $ cat test.cpp
  namespace ns {
void func();
  }
  ns::
  // complete
  $ c-index-test -code-completion-at=test.cpp:5:1 test.cpp
  FunctionDecl:{ResultType void}{TypedText func}{LeftParen (}{RightParen )} (50)
  Completion contexts:
  $ INDEXTEST_COMPLETION_CACHING=1 c-index-test 
-code-completion-at=test.cpp:5:1 test.cpp
  Completion contexts:




https://reviews.llvm.org/D40562



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

[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

This change breaks cached completions for declarations in namespaces in 
libclang. What exactly are you trying to achieve here? We could introduce 
another flag maybe.


https://reviews.llvm.org/D40562



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

alexfh wrote:
> This is not about the check, rather about the underlying style guide. The 
> document linked here doesn't explain why certain features are disallowed. I'd 
> suggest putting some effort in expanding the document to include reasoning 
> for each rule (e.g. see 
> https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance for a 
> related rule in the Google C++ style guide).
Good point -- we're looking into updating it. Thanks!


https://reviews.llvm.org/D40580



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124853.
juliehockett added a comment.

Rebasing for updated Release Notes -- so much nicer :)


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -135,6 +135,11 @@
 
   Warns if a function or method is declared or called with default arguments.
 

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> If the type extension approach is proven to be sound

I lack the full context here, but in my experience Z3 is really great for 
proving whether certain operations may or may not overflow, using the built-in 
bitvector type.
(I'm not sure though if that is what is being done here).


https://reviews.llvm.org/D35109



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

r319388 - [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via cfe-commits]

Author: dberris
Date: Wed Nov 29 16:04:54 2017
New Revision: 319388

URL: http://llvm.org/viewvc/llvm-project?rev=319388=rev
Log:
[XRay][clang] Introduce -fxray-always-emit-customevents

Summary:
The -fxray-always-emit-customevents flag instructs clang to always emit
the LLVM IR for calls to the `__xray_customevent(...)` built-in
function. The default behaviour currently respects whether the function
has an `[[clang::xray_never_instrument]]` attribute, and thus not lower
the appropriate IR code for the custom event built-in.

This change allows users calling through to the
`__xray_customevent(...)` built-in to always see those calls lowered to
the corresponding LLVM IR to lay down instrumentation points for these
custom event calls.

Using this flag enables us to emit even just the user-provided custom
events even while never instrumenting the start/end of the function
where they appear. This is useful in cases where "phase markers" using
__xray_customevent(...) can have very few instructions, must never be
instrumented when entered/exited.

Reviewers: rnk, dblaikie, kpw

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/XRayArgs.h
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Driver/XRayArgs.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=319388=319387=319388=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Wed Nov 29 16:04:54 2017
@@ -272,6 +272,9 @@ LANGOPT(SanitizeAddressFieldPadding, 2,
"aggressive, 2: more aggressive)")
 
 LANGOPT(XRayInstrument, 1, 0, "controls whether to do XRay instrumentation")
+LANGOPT(XRayAlwaysEmitCustomEvents, 1, 0,
+"controls whether to always emit intrinsic calls to "
+"__xray_customevent(...) builtin.")
 
 BENIGN_LANGOPT(AllowEditorPlaceholders, 1, 0,
"allow editor placeholders in source")

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=319388=319387=319388=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Nov 29 16:04:54 2017
@@ -1060,6 +1060,12 @@ def fxray_never_instrument :
   Group, Flags<[CC1Option]>,
   HelpText<"Filename defining the whitelist for imbuing the 'never instrument' 
XRay attribute.">;
 
+def fxray_always_emit_customevents : Flag<["-"], 
"fxray-always-emit-customevents">, Group,
+  Flags<[CC1Option]>,
+  HelpText<"Determine whether to always emit __xray_customevent(...) calls 
even if the function it appears in is not always instrumented.">;
+def fnoxray_always_emit_customevents : Flag<["-"], 
"fno-xray-always-emit-customevents">, Group,
+  Flags<[CC1Option]>;
+
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group, Flags<[CC1Option]>,
   HelpText<"Use separate accesses for bitfields with legal widths and 
alignments.">;

Modified: cfe/trunk/include/clang/Driver/XRayArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/XRayArgs.h?rev=319388=319387=319388=diff
==
--- cfe/trunk/include/clang/Driver/XRayArgs.h (original)
+++ cfe/trunk/include/clang/Driver/XRayArgs.h Wed Nov 29 16:04:54 2017
@@ -24,6 +24,7 @@ class XRayArgs {
   std::vector ExtraDeps;
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
+  bool XRayAlwaysEmitCustomEvents = false;
 
 public:
   /// Parses the XRay arguments from an argument list.

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=319388=319387=319388=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Wed Nov 29 16:04:54 2017
@@ -84,6 +84,9 @@ CODEGENOPT(InstrumentFunctionEntryBare ,
 CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
 
+///< Set when -fxray-always-emit-customevents is enabled.
+CODEGENOPT(XRayAlwaysEmitCustomEvents , 

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319388: [XRay][clang] Introduce 
-fxray-always-emit-customevents (authored by dberris).

Changed prior to commit:
  https://reviews.llvm.org/D40601?vs=124842=124846#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40601

Files:
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/XRayArgs.h
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Driver/XRayArgs.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1060,6 +1060,12 @@
   Group, Flags<[CC1Option]>,
   HelpText<"Filename defining the whitelist for imbuing the 'never instrument' XRay attribute.">;
 
+def fxray_always_emit_customevents : Flag<["-"], "fxray-always-emit-customevents">, Group,
+  Flags<[CC1Option]>,
+  HelpText<"Determine whether to always emit __xray_customevent(...) calls even if the function it appears in is not always instrumented.">;
+def fnoxray_always_emit_customevents : Flag<["-"], "fno-xray-always-emit-customevents">, Group,
+  Flags<[CC1Option]>;
+
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group, Flags<[CC1Option]>,
   HelpText<"Use separate accesses for bitfields with legal widths and alignments.">;
Index: cfe/trunk/include/clang/Driver/XRayArgs.h
===
--- cfe/trunk/include/clang/Driver/XRayArgs.h
+++ cfe/trunk/include/clang/Driver/XRayArgs.h
@@ -24,6 +24,7 @@
   std::vector ExtraDeps;
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
+  bool XRayAlwaysEmitCustomEvents = false;
 
 public:
   /// Parses the XRay arguments from an argument list.
Index: cfe/trunk/include/clang/Basic/LangOptions.def
===
--- cfe/trunk/include/clang/Basic/LangOptions.def
+++ cfe/trunk/include/clang/Basic/LangOptions.def
@@ -272,6 +272,9 @@
"aggressive, 2: more aggressive)")
 
 LANGOPT(XRayInstrument, 1, 0, "controls whether to do XRay instrumentation")
+LANGOPT(XRayAlwaysEmitCustomEvents, 1, 0,
+"controls whether to always emit intrinsic calls to "
+"__xray_customevent(...) builtin.")
 
 BENIGN_LANGOPT(AllowEditorPlaceholders, 1, 0,
"allow editor placeholders in source")
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -84,6 +84,9 @@
 CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
 
+///< Set when -fxray-always-emit-customevents is enabled.
+CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0)
+
 ///< Set the minimum number of instructions in a function to determine selective
 ///< XRay instrumentation.
 VALUE_CODEGENOPT(XRayInstructionThreshold , 32, 200)
Index: cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
===
--- cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
+++ cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-always-emit-customevents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  static constexpr char kPhase[] = "never";
+  __xray_customevent(kPhase, 5);
+  // CHECK: call void @llvm.xray.customevent(i8*{{.*}}, i32 5)
+}
Index: cfe/trunk/lib/Driver/XRayArgs.cpp
===
--- cfe/trunk/lib/Driver/XRayArgs.cpp
+++ cfe/trunk/lib/Driver/XRayArgs.cpp
@@ -27,8 +27,6 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
-constexpr char XRayAlwaysInstrumentOption[] = "-fxray-always-instrument=";
-constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain , const ArgList ) {
@@ -63,6 +61,14 @@
 D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
 }
 
+// By default, the back-end will not emit the lowering for XRay customevent
+// calls if the function is not instrumented. 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/ASTContext.cpp:2200
+  Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+  CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+  if (BaseOffset != CurOffset)

rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > On reflection, I don't think this is right, and likewise I don't think 
> > > the check for unique object representations of the base class above is 
> > > quite right.
> > > 
> > > A class can be standard-layout but not POD for the purposes of layout 
> > > (eg, by declaring a special member function). If so, the derived class 
> > > can reuse the base class's tail padding, and if it does, the derived 
> > > class can have unique object representations even when the base class 
> > > does not. Example:
> > > 
> > > ```
> > > struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
> > > ```
> > > 
> > > Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique 
> > > object representations even though its base class `A` does not.
> > > 
> > > 
> > > This should be fairly easy to fix. One approach would be to change the 
> > > recursive call to `hasUniqueObjectRepresentations` above to return the 
> > > actual size occupied by the struct and its fields (rather than checking 
> > > for tail padding), and add that size here instead of using the base size. 
> > > Or you could query the DataSize in the record layout rather than getting 
> > > the (complete object) type size (but you'd then still need to check that 
> > > there's no bits of tail padding before the end of the dsize, since we 
> > > still pad out trailing bit-fields in the dsize computation).
> > According to the standard, the above case isn't, because it is non-trivial, 
> > right?  9.1 requires that "T" (B in your case) be trivially copyable, which 
> > it isn't, right?
> > 
> > The predicate condition for a template specialization
> > has_unique_object_representations shall be
> > satisfied if and only if:
> > (9.1) - T is trivially copyable, and
> > (9.2) - any two objects of type T with the same value have the same
> > object representation
> Fixed example:
> 
> ```
> struct A { int  char a; }; struct B : A { char b[7]; };
> ```
AH!  I got caught up by the destructor as the reason it wasn't unique, but the 
actual thing you meant is that "A" has tail padding, so it is NOT unique.  
However, on Itanium, the tail padding gets used when inheriting from it.

Do I have that correct? I just have to fix the behavior of 
inheriting-removing-tail-padding?


https://reviews.llvm.org/D39347



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

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/AST/ASTContext.cpp:2200
+  Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+  CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+  if (BaseOffset != CurOffset)

erichkeane wrote:
> rsmith wrote:
> > On reflection, I don't think this is right, and likewise I don't think the 
> > check for unique object representations of the base class above is quite 
> > right.
> > 
> > A class can be standard-layout but not POD for the purposes of layout (eg, 
> > by declaring a special member function). If so, the derived class can reuse 
> > the base class's tail padding, and if it does, the derived class can have 
> > unique object representations even when the base class does not. Example:
> > 
> > ```
> > struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
> > ```
> > 
> > Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique 
> > object representations even though its base class `A` does not.
> > 
> > 
> > This should be fairly easy to fix. One approach would be to change the 
> > recursive call to `hasUniqueObjectRepresentations` above to return the 
> > actual size occupied by the struct and its fields (rather than checking for 
> > tail padding), and add that size here instead of using the base size. Or 
> > you could query the DataSize in the record layout rather than getting the 
> > (complete object) type size (but you'd then still need to check that 
> > there's no bits of tail padding before the end of the dsize, since we still 
> > pad out trailing bit-fields in the dsize computation).
> According to the standard, the above case isn't, because it is non-trivial, 
> right?  9.1 requires that "T" (B in your case) be trivially copyable, which 
> it isn't, right?
> 
> The predicate condition for a template specialization
> has_unique_object_representations shall be
> satisfied if and only if:
> (9.1) - T is trivially copyable, and
> (9.2) - any two objects of type T with the same value have the same
> object representation
Fixed example:

```
struct A { int  char a; }; struct B : A { char b[7]; };
```


https://reviews.llvm.org/D39347



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

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 124842.
dberris marked an inline comment as done.
dberris edited the summary of this revision.
dberris added a comment.

Updated description, address review comment(s).


https://reviews.llvm.org/D40601

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-always-emit-customevent.cpp

Index: clang/test/CodeGen/xray-always-emit-customevent.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-always-emit-customevent.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-always-emit-customevents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  static constexpr char kPhase[] = "never";
+  __xray_customevent(kPhase, 5);
+  // CHECK: call void @llvm.xray.customevent(i8*{{.*}}, i32 5)
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -785,6 +785,8 @@
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
@@ -2503,6 +2505,11 @@
   Opts.XRayInstrument =
   Args.hasFlag(OPT_fxray_instrument, OPT_fnoxray_instrument, false);
 
+  // -fxray-always-emit-customevents
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasFlag(OPT_fxray_always_emit_customevents,
+   OPT_fnoxray_always_emit_customevents, false);
+
   // -fxray-{always,never}-instrument= filenames.
   Opts.XRayAlwaysInstrumentFiles =
   Args.getAllArgValues(OPT_fxray_always_instrument);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -27,8 +27,6 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
-constexpr char XRayAlwaysInstrumentOption[] = "-fxray-always-instrument=";
-constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain , const ArgList ) {
@@ -63,6 +61,14 @@
 D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
 }
 
+// By default, the back-end will not emit the lowering for XRay customevent
+// calls if the function is not instrumented. In the future we will change
+// this default to be the reverse, but in the meantime we're going to
+// introduce the new functionality behind a flag.
+if (Args.hasFlag(options::OPT_fxray_always_emit_customevents,
+ options::OPT_fnoxray_always_emit_customevents, false))
+  XRayAlwaysEmitCustomEvents = true;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto  :
@@ -91,17 +97,21 @@
 return;
 
   CmdArgs.push_back(XRayInstrumentOption);
+
+  if (XRayAlwaysEmitCustomEvents)
+CmdArgs.push_back("-fxray-always-emit-customevents");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
   for (const auto  : AlwaysInstrumentFiles) {
-SmallString<64> AlwaysInstrumentOpt(XRayAlwaysInstrumentOption);
+SmallString<64> AlwaysInstrumentOpt("-fxray-always-instrument=");
 AlwaysInstrumentOpt += Always;
 CmdArgs.push_back(Args.MakeArgString(AlwaysInstrumentOpt));
   }
 
   for (const auto  : NeverInstrumentFiles) {
-SmallString<64> NeverInstrumentOpt(XRayNeverInstrumentOption);
+SmallString<64> NeverInstrumentOpt("-fxray-never-instrument=");
 NeverInstrumentOpt += Never;
 CmdArgs.push_back(Args.MakeArgString(NeverInstrumentOpt));
   }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1771,6 +1771,10 @@
   /// instrumented with XRay nop sleds.
   bool ShouldXRayInstrumentFunction() const;
 
+  /// AlwaysEmitXRayCustomEvents - Return true 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: lib/AST/ASTContext.cpp:2200
+  Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+  CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+  if (BaseOffset != CurOffset)

rsmith wrote:
> On reflection, I don't think this is right, and likewise I don't think the 
> check for unique object representations of the base class above is quite 
> right.
> 
> A class can be standard-layout but not POD for the purposes of layout (eg, by 
> declaring a special member function). If so, the derived class can reuse the 
> base class's tail padding, and if it does, the derived class can have unique 
> object representations even when the base class does not. Example:
> 
> ```
> struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
> ```
> 
> Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique object 
> representations even though its base class `A` does not.
> 
> 
> This should be fairly easy to fix. One approach would be to change the 
> recursive call to `hasUniqueObjectRepresentations` above to return the actual 
> size occupied by the struct and its fields (rather than checking for tail 
> padding), and add that size here instead of using the base size. Or you could 
> query the DataSize in the record layout rather than getting the (complete 
> object) type size (but you'd then still need to check that there's no bits of 
> tail padding before the end of the dsize, since we still pad out trailing 
> bit-fields in the dsize computation).
According to the standard, the above case isn't, because it is non-trivial, 
right?  9.1 requires that "T" (B in your case) be trivially copyable, which it 
isn't, right?

The predicate condition for a template specialization
has_unique_object_representations shall be
satisfied if and only if:
(9.1) - T is trivially copyable, and
(9.2) - any two objects of type T with the same value have the same
object representation


https://reviews.llvm.org/D39347



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

[PATCH] D40621: MS ABI: Treat explicit instantiation definitions of dllimport function templates as explicit instantiation decls (PR35435)

[Hans Wennborg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319386: MS ABI: Treat explicit instantiation definitions of 
dllimport function… (authored by hans).

Changed prior to commit:
  https://reviews.llvm.org/D40621?vs=124808=124839#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40621

Files:
  cfe/trunk/lib/Sema/SemaTemplate.cpp
  cfe/trunk/test/CodeGenCXX/dllimport.cpp


Index: cfe/trunk/test/CodeGenCXX/dllimport.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp
@@ -579,7 +579,7 @@
 // MSC-DAG: declare dllimport void 
@"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GNU-DAG: declare dllimport void @_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GNU-DAG: define weak_odr void 
@_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
-// MO1-DAG: define available_externally dllimport void 
@"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
+// MO1-DAG: declare dllimport void 
@"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
 // MO1-DAG: define available_externally dllimport void 
@"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GO1-DAG: define available_externally dllimport void 
@_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GO1-DAG: define weak_odr void 
@_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
@@ -609,6 +609,15 @@
 template<> __declspec(dllimport) inline void 
funcTmpl() {}
 USE(funcTmpl)
 
+#ifdef MSABI
+namespace pr35435 {
+struct X;
+template  struct __declspec(dllimport) S {
+  void foo(T *t) { t->problem(); }
+};
+template void S::foo(X*); // Cannot be instantiated because X is 
incomplete; dllimport means it's treated as an instantiation decl.
+}
+#endif
 
 
 
//===--===//
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -9239,10 +9239,18 @@
   return (Decl*) nullptr;
   }
 
-  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
   if (Attr)
 ProcessDeclAttributeList(S, Specialization, Attr);
 
+  // In MSVC mode, dllimported explicit instantiation definitions are treated 
as
+  // instantiation declarations.
+  if (TSK == TSK_ExplicitInstantiationDefinition &&
+  Specialization->hasAttr() &&
+  Context.getTargetInfo().getCXXABI().isMicrosoft())
+TSK = TSK_ExplicitInstantiationDeclaration;
+
+  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
+
   if (Specialization->isDefined()) {
 // Let the ASTConsumer know that this function has been explicitly
 // instantiated now, and its linkage might have changed.


Index: cfe/trunk/test/CodeGenCXX/dllimport.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp
@@ -579,7 +579,7 @@
 // MSC-DAG: declare dllimport void @"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GNU-DAG: declare dllimport void @_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GNU-DAG: define weak_odr void @_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
-// MO1-DAG: define available_externally dllimport void @"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
+// MO1-DAG: declare dllimport void @"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
 // MO1-DAG: define available_externally dllimport void @"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GO1-DAG: define available_externally dllimport void @_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GO1-DAG: define weak_odr void @_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
@@ -609,6 +609,15 @@
 template<> __declspec(dllimport) inline void funcTmpl() {}
 USE(funcTmpl)
 
+#ifdef MSABI
+namespace pr35435 {
+struct X;
+template  struct __declspec(dllimport) S {
+  void foo(T *t) { t->problem(); }
+};
+template void S::foo(X*); // Cannot be instantiated because X is incomplete; dllimport means it's treated as an instantiation decl.
+}
+#endif
 
 
 //===--===//
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -9239,10 +9239,18 @@
   return (Decl*) nullptr;
   }
 
-  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
   if (Attr)
 ProcessDeclAttributeList(S, Specialization, Attr);
 
+  // In MSVC mode, dllimported explicit instantiation definitions are treated as
+  // instantiation declarations.
+  if (TSK == TSK_ExplicitInstantiationDefinition &&
+  Specialization->hasAttr() &&
+  Context.getTargetInfo().getCXXABI().isMicrosoft())
+TSK = TSK_ExplicitInstantiationDeclaration;
+
+  

r319386 - MS ABI: Treat explicit instantiation definitions of dllimport function templates as explicit instantiation decls (PR35435)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Nov 29 15:44:11 2017
New Revision: 319386

URL: http://llvm.org/viewvc/llvm-project?rev=319386=rev
Log:
MS ABI: Treat explicit instantiation definitions of dllimport function 
templates as explicit instantiation decls (PR35435)

This matches MSVC's behaviour, and we already do it for class templates
since r270897.

Differential revision: https://reviews.llvm.org/D40621

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=319386=319385=319386=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Nov 29 15:44:11 2017
@@ -9239,10 +9239,18 @@ DeclResult Sema::ActOnExplicitInstantiat
   return (Decl*) nullptr;
   }
 
-  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
   if (Attr)
 ProcessDeclAttributeList(S, Specialization, Attr);
 
+  // In MSVC mode, dllimported explicit instantiation definitions are treated 
as
+  // instantiation declarations.
+  if (TSK == TSK_ExplicitInstantiationDefinition &&
+  Specialization->hasAttr() &&
+  Context.getTargetInfo().getCXXABI().isMicrosoft())
+TSK = TSK_ExplicitInstantiationDeclaration;
+
+  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
+
   if (Specialization->isDefined()) {
 // Let the ASTConsumer know that this function has been explicitly
 // instantiated now, and its linkage might have changed.

Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=319386=319385=319386=diff
==
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Wed Nov 29 15:44:11 2017
@@ -579,7 +579,7 @@ USE(inlineFuncTmpl)
 template<> __declspec(dllimport) inline void 
funcTmpl() {}
 USE(funcTmpl)
 
+#ifdef MSABI
+namespace pr35435 {
+struct X;
+template  struct __declspec(dllimport) S {
+  void foo(T *t) { t->problem(); }
+};
+template void S::foo(X*); // Cannot be instantiated because X is 
incomplete; dllimport means it's treated as an instantiation decl.
+}
+#endif
 
 
 
//===--===//


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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1071
 
+Location getDeclarationLocation(ParsedAST ,
+const SourceRange ) {

llvm::ErrorOr



Comment at: clangd/ClangdUnit.cpp:1075
+  const LangOptions  = AST.getASTContext().getLangOpts();
+  SourceLocation LocStart = ValSourceRange.getBegin();
+  SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 
0,

To fix the macro expansion crash:
```
SourceLocation LocStart = SourceMgr.getExpansionLoc(ValSourceRange.getBegin());
const FileEntry *F =
SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));
```
and simple early return
```
if (!F)
  return llvm::errc::no_such_file_or_directory;
```



Comment at: clangd/ClangdUnit.cpp:1079
+  Position Begin;
+  Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
+  Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;

getExpansionLineNumber



Comment at: clangd/ClangdUnit.cpp:1080
+  Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
+  Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
+  Position End;

getExpansionColumnNumber



Comment at: clangd/ClangdUnit.cpp:1082
+  Position End;
+  End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
+  End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;

getExpansionLineNumber



Comment at: clangd/ClangdUnit.cpp:1083
+  End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
+  End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
+  Range R = {Begin, End};

getExpansionColumnNumber



Comment at: clangd/ClangdUnit.cpp:1087
+
+  const FileEntry *F =
+  SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));

You can do this earlier and simpler, see comment above.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894



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

[PATCH] D38425: [clangd] Document highlights for clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 124834.
Nebiroth added a comment.

Fixed wrong content header making the test fail


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38425

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/documenthighlight.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/documenthighlight.test
===
--- /dev/null
+++ test/clangd/documenthighlight.test
@@ -0,0 +1,42 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 479
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};\nstruct Foo {\nint xasd;\n};\n}\nint main() {\nint bonjour;\nbonjour = 2;\nint test1 = bonjour;\nns1::Foo bar = { xasd : 1};\nbar.xasd = 3;\nns1::MyClass* Params;\nParams->anotherOperation();\n}\n"}}}
+
+Content-Length: 156
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":2}}}
+# Verify local variable 
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":17}}}
+# Verify struct highlight
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":10}}}
+# Verify method highlight
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":14,"line":2},"start":{"character":7,"line":2}}},{"kind":1,"range":{"end":{"character":22,"line":6},"start":{"character":15,"line":6}}},{"kind":1,"range":{"end":{"character":12,"line":21},"start":{"character":5,"line":21}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":14}}}
+# Verify Read-access of a symbol (kind = 2) 
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}			
\ No newline at end of file
Index: clangd/ProtocolHandlers.h

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle requested changes to this revision.
malaperle added inline comments.
This revision now requires changes to proceed.



Comment at: clangd/Protocol.cpp:498
+// Default Hover values
+Hover H;
+return json::obj{

remove, we have to return the contents of the H that was passed as parameter, 
not a new one. I hit this bug while testing with no range (hover text in 
another file)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894



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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1117
+
+  if (!DeclVector.empty()) {
+const Decl *LocationDecl = DeclVector[0];

malaperle wrote:
> I think we need to simplify this part a bit. I'll try to find a way. Feel 
> free to wait until more comments before updating.
Here's the version in which I tried to simplify this *a bit*. With the new 
ErrorOr checks as well.

```
Hover clangd::getHover(ParsedAST , Position Pos, clangd::Logger ) {
  const SourceManager  = AST.getASTContext().getSourceManager();
  const LangOptions  = AST.getASTContext().getLangOpts();
  const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
  if (!FE)
return Hover();

  SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);

  auto DeclMacrosFinder = std::make_shared(
  llvm::errs(), SourceLocationBeg, AST.getASTContext(),
  AST.getPreprocessor());
  index::IndexingOptions IndexOpts;
  IndexOpts.SystemSymbolFilter =
  index::IndexingOptions::SystemSymbolFilterKind::All;
  IndexOpts.IndexFunctionLocals = true;
  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
 DeclMacrosFinder, IndexOpts);

  auto Decls = DeclMacrosFinder->takeDecls();
  if (!Decls.empty() && Decls[0]) {
const Decl *LocationDecl = Decls[0];
std::vector HoverContents;

// Compute scope as the first "section" of the hover.
if (const NamedDecl *NamedD = dyn_cast(LocationDecl)) {
  std::string Scope = getScope(NamedD, AST.getASTContext().getLangOpts());
  if (!Scope.empty()) {
MarkedString Info = {"", "C++", "In " + Scope};
HoverContents.push_back(Info);
  }
}

// Adjust beginning of hover text depending on the presence of templates 
and comments.
TemplateDecl * TDec = LocationDecl->getDescribedTemplate();
const Decl * BeginDecl = TDec ? TDec : LocationDecl;
SourceLocation BeginLocation = BeginDecl->getSourceRange().getBegin();
RawComment *RC = AST.getASTContext().getRawCommentForDeclNoCache(
BeginDecl);
if (RC)
  BeginLocation = RC->getLocStart();

// Adjust end of hover text for things that have braces/bodies. We don't
// want to show the whole definition of a function, class, etc.
SourceLocation EndLocation = LocationDecl->getSourceRange().getEnd();
if (auto FuncDecl = dyn_cast(LocationDecl)) {
  if (FuncDecl->isThisDeclarationADefinition() && FuncDecl->getBody())
EndLocation = FuncDecl->getBody()->getLocStart();
} else if (auto TagDeclaration = dyn_cast(LocationDecl)) {
  if (TagDeclaration->isThisDeclarationADefinition())
EndLocation = TagDeclaration->getBraceRange().getBegin();
} else if (auto NameDecl = dyn_cast(LocationDecl)) {
  SourceLocation BeforeLBraceLoc = Lexer::getLocForEndOfToken(
  LocationDecl->getLocation(), 0, SourceMgr, LangOpts);
  if (BeforeLBraceLoc.isValid())
EndLocation = BeforeLBraceLoc;
}

SourceRange SR(BeginLocation, EndLocation);
if (SR.isValid()) {
  auto L = getDeclarationLocation(AST, SR);
  if (L) {
auto Ref = getBufferDataFromSourceRange(AST, *L);
if (Ref) {
  Hover H;
  if (SourceMgr.isInMainFile(BeginLocation))
H.range = L->range;
  MarkedString MS = {"", "C++", *Ref};
  HoverContents.push_back(MS);
  H.contents = std::move(HoverContents);
  return H;
}
  }
}
  }

  auto MacroInfos = DeclMacrosFinder->takeMacroInfos();
  if (!MacroInfos.empty() && MacroInfos[0]) {
const MacroInfo *MacroInf = MacroInfos[0];
SourceRange SR(MacroInf->getDefinitionLoc(),
 MacroInf->getDefinitionEndLoc());
if (SR.isValid()) {
  auto L = getDeclarationLocation(AST, SR);
  if (L) {
auto Ref = getBufferDataFromSourceRange(AST, *L);
if (Ref) {
  Hover H;
  if (SourceMgr.isInMainFile(SR.getBegin()))
H.range = L->range;
  MarkedString MS = {"", "C++", "#define " + Ref->str()};
  H.contents.push_back(MS);
  return H;
}
  }
}
  }

  return Hover();
}
```



Comment at: clangd/ClangdUnit.cpp:1172
+Location L = getDeclarationLocation(AST, SR);
+H.range = L.range;
+Ref = getDataBufferFromSourceRange(AST, SR, L);

The range could be in another file but we can only highlight things in the file 
that the user current has open. For example, if I'm foo.cpp and I hover on 
something declared in foo.h, it will change the background color in foo.cpp but 
using the line numbers of foo.h! The protocol should be more clear about this 
but since there are no Uri in the Hover struct, we can assume this range should 
only apply to the open file, i.e. the main file. So I suggest this check:
  if (SourceMgr.isInMainFile(BeginLocation))
H.range = 

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/AST/ASTContext.cpp:2200
+  Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+  CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+  if (BaseOffset != CurOffset)

On reflection, I don't think this is right, and likewise I don't think the 
check for unique object representations of the base class above is quite right.

A class can be standard-layout but not POD for the purposes of layout (eg, by 
declaring a special member function). If so, the derived class can reuse the 
base class's tail padding, and if it does, the derived class can have unique 
object representations even when the base class does not. Example:

```
struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
```

Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique object 
representations even though its base class `A` does not.


This should be fairly easy to fix. One approach would be to change the 
recursive call to `hasUniqueObjectRepresentations` above to return the actual 
size occupied by the struct and its fields (rather than checking for tail 
padding), and add that size here instead of using the base size. Or you could 
query the DataSize in the record layout rather than getting the (complete 
object) type size (but you'd then still need to check that there's no bits of 
tail padding before the end of the dsize, since we still pad out trailing 
bit-fields in the dsize computation).



Comment at: lib/AST/ASTContext.cpp:2231
+
+  // Handles tail-padding.  >= comparision takes care of EBO cases.
+  return CurOffsetInBits == Context.toBits(Layout.getSize());

What `>=` comparison? I think the comment has got out of date relative to the 
code.


https://reviews.llvm.org/D39347



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

[PATCH] D40198: [CUDA] Tweak CUDA wrappers to make cuda-9 work with libc++

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

ping.


https://reviews.llvm.org/D40198



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

[PATCH] D35894: [clangd] Code hover for Clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 124833.
Nebiroth added a comment.
Herald added a subscriber: klimek.

Invalid FileEntries now return llvm:ErrorOr


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/hover.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -30,6 +30,7 @@
 # CHECK-NEXT:  "clangd.applyFix"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:	   "hoverProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -30,6 +30,7 @@
 # CHECK-NEXT:  "clangd.applyFix"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:	   "hoverProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
Index: test/clangd/hover.test
===
--- /dev/null
+++ test/clangd/hover.test
@@ -0,0 +1,56 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 611
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nint test = 5;\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};}\n//comments test\ntemplate\nT templateTest(T foo) {\nreturn foo;}\ntemplate\nclass classTemplateTest {\npublic: T test;};\nint main() {\nint a;\na;\nint b = ns1::test;\nns1::MyClass* Params;\nParams->anotherOperation();\nMACRO;\nint temp = 5;\ntemplateTest(temp);classTemplateTest test;}\n"}}}
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":0,"character":12}}}
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"#define MACRO 1"}],"range":{"end":{"character":15,"line":0},"start":{"character":8,"line":0
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":1}}}
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"int a"}],"range":{"end":{"character":5,"line":20},"start":{"character":0,"line":20
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":22,"character":15}}}
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"In ns1"},{"language":"C++","value":"int test = 5"}],"range":{"end":{"character":12,"line":2},"start":{"character":0,"line":2
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":23,"character":10}}}
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"In ns1"},{"language":"C++","value":"struct MyClass {"}],"range":{"end":{"character":16,"line":3},"start":{"character":0,"line":3
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":24,"character":13}}}
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"In ns1::MyClass"},{"language":"C++","value":"void anotherOperation() {"}],"range":{"end":{"character":25,"line":5},"start":{"character":0,"line":5
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":25,"character":1}}}
+# CHECK: {"id":1,"jsonrpc":"2.0","result":{"contents":[{"language":"C++","value":"#define MACRO 1"}],"range":{"end":{"character":15,"line":0},"start":{"character":8,"line":0
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":26,"character":8}}}
+# 

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Aaron Ballman via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D40574#939693, @mattd wrote:

> Thanks for the approval Aaron.  I do not have commit access, would you mind 
> submitting this on my behalf?


Happy to do so -- I've commit in r319383, thank you for the patch!


https://reviews.llvm.org/D40574



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

r319383 - Perform a bounds check on a function's argument list before accessing any index value specified by an 'argument_with_type_tag' attribute. Fixes PR28520.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Nov 29 15:10:14 2017
New Revision: 319383

URL: http://llvm.org/viewvc/llvm-project?rev=319383=rev
Log:
Perform a bounds check on a function's argument list before accessing any index 
value specified by an 'argument_with_type_tag' attribute. Fixes PR28520.

Patch by Matt Davis.

Added:
cfe/trunk/test/Sema/error-type-safety.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=319383=319382=319383=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 29 15:10:14 
2017
@@ -7919,6 +7919,8 @@ def err_type_tag_for_datatype_too_large
   "'type_tag_for_datatype' attribute requires the initializer to be "
   "an %select{integer|integral}0 constant expression "
   "that can be represented by a 64 bit integer">;
+def err_tag_index_out_of_range : Error<
+  "%select{type tag|argument}0 index %1 is greater than the number of 
arguments specified">;
 def warn_type_tag_for_datatype_wrong_kind : Warning<
   "this type tag was not designed to be used with this function">,
   InGroup;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=319383=319382=319383=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Nov 29 15:10:14 2017
@@ -10455,7 +10455,8 @@ private:
   /// \brief Peform checks on a call of a function with argument_with_type_tag
   /// or pointer_with_type_tag attributes.
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
 
   /// \brief Check if we are taking the address of a packed field
   /// as this may be a problem if the pointer value is dereferenced.

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=319383=319382=319383=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Nov 29 15:10:14 2017
@@ -2754,7 +2754,7 @@ void Sema::checkCall(NamedDecl *FDecl, c
 // Type safety checking.
 if (FDecl) {
   for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
 }
   }
 
@@ -12329,10 +12329,18 @@ static bool IsSameCharType(QualType T1,
 }
 
 void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
   const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
   bool IsPointerAttr = Attr->getIsPointer();
 
+  // Retrieve the argument representing the 'type_tag'.
+  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 0 << Attr->getTypeTagIdx() + 1;
+return;
+  }
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
   bool FoundWrongKind;
   TypeTagData TypeInfo;
@@ -12346,6 +12354,13 @@ void Sema::CheckArgumentWithTypeTag(cons
 return;
   }
 
+  // Retrieve the argument representing the 'arg_idx'.
+  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 1 << Attr->getArgumentIdx() + 1;
+return;
+  }
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
   if (IsPointerAttr) {
 // Skip implicit cast of pointer to `void *' (as a function argument).

Added: cfe/trunk/test/Sema/error-type-safety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/error-type-safety.cpp?rev=319383=auto
==
--- cfe/trunk/test/Sema/error-type-safety.cpp (added)
+++ cfe/trunk/test/Sema/error-type-safety.cpp Wed Nov 29 15:10:14 2017
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define INT_TAG 42
+
+static const int test_in
+  __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
+
+// Argument index: 1, Type tag index: 2
+void test_bounds_index(...)
+  __attribute__((argument_with_type_tag(test, 1, 

[PATCH] D40625: Harmonizing attribute GNU/C++ spellings

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
Herald added subscribers: kristof.beyls, Anastasia, mehdi_amini, aemerson.

Based on discussions at the WG21 meeting in Albuquerque and follow-up email 
discussions, I believe the correct approach to exposing attributes from Clang 
is to provide them with GNU-style __attribute__ spellings as well as C++-style 
[[]] spellings in the clang vendor namespace, when appropriate. If the 
attribute was originally provided by a different vendor, and Clang intends to 
be compatible with those semantics, then the attribute should only be provided 
with the spellings supported by the original vendor. Otherwise, any attributes 
provided only by Clang should be exposed as both a GNU-style and C++-style 
attribute as appropriate for the semantics of that attribute.

To that end, this patch adds a C++ spelling in the clang vendor namespace to a 
number of attributes. Similarly, it adds a GNU spelling to the 
`lto_visibility_public` attribute (which was the only one with a C++ spelling 
but not a GNU spelling).

Finally, it leaves the following attributes with only a GNU spelling, based on 
the given rationale:

align_value -- originally provided by Intel with only a GNU spelling

constant, cudart_builtin, device, device_builtin_surface_type, 
device_builtin_texture_type, host, launch_bounds, shared, nv_weak -- attributes 
specified by CUDA with only a GNU spelling (several are also ignored attributes)

opencl_unroll_hint, intel_reqd_sub_group_size, nosvm, ext_vector_type, 
reqd_work_group_size, work_group_size_hint, vec_type_hint -- attributes 
specified by OpenCL with only a GNU spelling

kernel -- specified by RenderScript

carries_dependency -- supported as a standards-based attribute, shouldn't be in 
a vendor namespace

enable_if, diagnose_if, guarded_by, pt_guarded_by, acquired_after, 
acquired_before, assert_exclusive_lock, assert_shared_lock, 
exclusive_trylock_function, shared_trylock_function, lock_returned, 
locks_excluded -- not easily used with the C++ spelling because the attribute 
requires naming function parameters (these might be good candidates to explore 
changing into type attributes in the future).

bounded -- currently ignored

*Please double-check my understanding of these attribute spellings.* It is 
possible that I've misunderstood attributes as being specified by other sources 
when in fact they are Clang extensions, or that something is listed as a Clang 
extension but is actually a compatibility attribute. If my understanding is 
correct, I'll add the rationales to Attr.td as comments alongside the attribute 
spelling so that we don't lose that information.

Because this is a mechanical change that only introduces new spellings, there 
are no proposed test cases for the patch.


https://reviews.llvm.org/D40625

Files:
  include/clang/Basic/Attr.td

Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -518,7 +518,7 @@
 }
 
 def AddressSpace : TypeAttr {
-  let Spellings = [GNU<"address_space">];
+  let Spellings = [Clang<"address_space">];
   let Args = [IntArgument<"AddressSpace">];
   let Documentation = [Undocumented];
 }
@@ -599,12 +599,12 @@
 }
 
 def AnalyzerNoReturn : InheritableAttr {
-  let Spellings = [GNU<"analyzer_noreturn">];
+  let Spellings = [Clang<"analyzer_noreturn">];
   let Documentation = [Undocumented];
 }
 
 def Annotate : InheritableParamAttr {
-  let Spellings = [GNU<"annotate">];
+  let Spellings = [Clang<"annotate">];
   let Args = [StringArgument<"Annotation">];
   // Ensure that the annotate attribute can be used with
   // '#pragma clang attribute' even though it has no subject list.
@@ -646,7 +646,7 @@
 }
 
 def Availability : InheritableAttr {
-  let Spellings = [GNU<"availability">];
+  let Spellings = [Clang<"availability">];
   let Args = [IdentifierArgument<"platform">, VersionArgument<"introduced">,
   VersionArgument<"deprecated">, VersionArgument<"obsoleted">,
   BoolArgument<"unavailable">, StringArgument<"message">,
@@ -706,7 +706,7 @@
 }
 
 def Blocks : InheritableAttr {
-  let Spellings = [GNU<"blocks">];
+  let Spellings = [Clang<"blocks">];
   let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>];
   let Documentation = [Undocumented];
 }
@@ -733,34 +733,34 @@
 // cf_returns_retained attributes.  It is generally applied by
 // '#pragma clang arc_cf_code_audited' rather than explicitly.
 def CFAuditedTransfer : InheritableAttr {
-  let Spellings = [GNU<"cf_audited_transfer">];
+  let Spellings = [Clang<"cf_audited_transfer">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 
 // cf_unknown_transfer is an explicit opt-out of cf_audited_transfer.
 // It indicates that the function has unknown or unautomatable
 // transfer semantics.
 def CFUnknownTransfer : InheritableAttr {
-  let Spellings = 

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Devin Coughlin via Phabricator via cfe-commits]

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

Thanks, this looks good to me!

Do you have commit access or do you need someone to commit it for you?




Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

xazax.hun wrote:
> lebedev.ri wrote:
> > xazax.hun wrote:
> > > lebedev.ri wrote:
> > > > dcoughlin wrote:
> > > > > "Unary operator" is probably not something we should expect users to 
> > > > > know about. How about just "The expression is an uninitialized value. 
> > > > > The computed value will also be garbage."
> > > > > 
> > > > Yep, i did not like my wording either :)
> > > A value being undefined does nt mean uninitialized. E.g.: the result of a 
> > > bad shift operation might be UndefVal as well.
> > > Aren't these diagnostics misleading in those cases? Or we do not care 
> > > about those?
> > I think this is for @dcoughlin to answer.
> > But shift operation is a binary operator, so this diff should not change 
> > that.
> I was wondering about examples like:
> ```
> int x = 1 << -1;
> ++x;
> ```
> 
> In this particular case, it will not issue the misleading error message. The 
> shift that results in an undefined value will generate an error node, so we 
> will not warn for the pre increment.
> 
> But in general I am a bit uncomfortable about the assumption of this check: 
> if a value is undefined the reason is that it is being uninitialized. 
> 
> Note that, of course this problem is not specific to this patch but a general 
> question about the checker. 
It is a good point, but I think fixing it should wait until a later patch.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

r319376 - [AST] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed Nov 29 14:39:22 2017
New Revision: 319376

URL: http://llvm.org/viewvc/llvm-project?rev=319376=rev
Log:
[AST] Fix some Clang-tidy modernize and Include What You Use warnings; other 
minor fixes (NFC).

Modified:
cfe/trunk/include/clang/AST/DeclTemplate.h
cfe/trunk/lib/AST/DeclTemplate.cpp

Modified: cfe/trunk/include/clang/AST/DeclTemplate.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=319376=319375=319376=diff
==
--- cfe/trunk/include/clang/AST/DeclTemplate.h (original)
+++ cfe/trunk/include/clang/AST/DeclTemplate.h Wed Nov 29 14:39:22 2017
@@ -1,4 +1,4 @@
-//===-- DeclTemplate.h - Classes for representing C++ templates -*- C++ 
-*-===//
+//===- DeclTemplate.h - Classes for representing C++ templates --*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -6,42 +6,60 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-///
+//
 /// \file
 /// \brief Defines the C++ template declaration subclasses.
-///
+//
 
//===--===//
 
 #ifndef LLVM_CLANG_AST_DECLTEMPLATE_H
 #define LLVM_CLANG_AST_DECLTEMPLATE_H
 
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/Redeclarable.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/iterator.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/TrailingObjects.h"
+#include 
+#include 
+#include 
+#include 
 #include 
 
 namespace clang {
 
 enum BuiltinTemplateKind : int;
-class TemplateParameterList;
-class TemplateDecl;
-class RedeclarableTemplateDecl;
-class FunctionTemplateDecl;
 class ClassTemplateDecl;
 class ClassTemplatePartialSpecializationDecl;
-class TemplateTypeParmDecl;
+class Expr;
+class FunctionTemplateDecl;
+class IdentifierInfo;
 class NonTypeTemplateParmDecl;
+class TemplateDecl;
 class TemplateTemplateParmDecl;
-class TypeAliasTemplateDecl;
+class TemplateTypeParmDecl;
+class UnresolvedSetImpl;
 class VarTemplateDecl;
 class VarTemplatePartialSpecializationDecl;
 
 /// \brief Stores a template parameter of any kind.
-typedef llvm::PointerUnion3 TemplateParameter;
+using TemplateParameter =
+llvm::PointerUnion3;
 
 NamedDecl *getAsNamedDecl(TemplateParameter P);
 
@@ -50,7 +68,6 @@ NamedDecl *getAsNamedDecl(TemplateParame
 class TemplateParameterList final
 : private llvm::TrailingObjects {
-
   /// The location of the 'template' keyword.
   SourceLocation TemplateLoc;
 
@@ -69,6 +86,10 @@ class TemplateParameterList final
   unsigned HasRequiresClause : 1;
 
 protected:
+  TemplateParameterList(SourceLocation TemplateLoc, SourceLocation LAngleLoc,
+ArrayRef Params, SourceLocation RAngleLoc,
+Expr *RequiresClause);
+
   size_t numTrailingObjects(OverloadToken) const {
 return NumParams;
   }
@@ -77,11 +98,11 @@ protected:
 return HasRequiresClause;
   }
 
-  TemplateParameterList(SourceLocation TemplateLoc, SourceLocation LAngleLoc,
-ArrayRef Params, SourceLocation RAngleLoc,
-Expr *RequiresClause);
-
 public:
+  template 
+  friend class FixedSizeTemplateParameterListStorage;
+  friend TrailingObjects;
+
   static TemplateParameterList *Create(const ASTContext ,
SourceLocation TemplateLoc,
SourceLocation LAngleLoc,
@@ -90,10 +111,10 @@ public:
Expr *RequiresClause);
 
   /// \brief Iterates through the template parameters in this list.
-  typedef NamedDecl** iterator;
+  using iterator = NamedDecl **;
 
   /// \brief Iterates through the template parameters in this list.
-  typedef NamedDecl* const* const_iterator;
+  using const_iterator = NamedDecl * const *;
 
   iterator begin() { return getTrailingObjects(); }
   const_iterator begin() const { return getTrailingObjects(); }
@@ -113,7 +134,6 @@ public:
 assert(Idx < size() && "Template parameter index out-of-range");
 return begin()[Idx];
   }
-
   const NamedDecl* getParam(unsigned Idx) const {
 assert(Idx < size() && "Template parameter index out-of-range");
 return 

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please rebase from trunk. I just enforced some order in chaos of Release Notes 
:-)


https://reviews.llvm.org/D40580



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

r319373 - [Coverage] Emit gap areas in braces-optional statements (PR35387)

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Wed Nov 29 14:25:14 2017
New Revision: 319373

URL: http://llvm.org/viewvc/llvm-project?rev=319373=rev
Log:
[Coverage] Emit gap areas in braces-optional statements (PR35387)

Emit a gap area starting after the r-paren location and ending at the
start of the body for the braces-optional statements (for, for-each,
while, etc). The count for the gap area equal to the body's count. This
extends the fix in r317758.

Fixes PR35387, rdar://35570345

Testing: stage2 coverage-enabled build of clang, check-clang

Modified:
cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp
cfe/trunk/test/CoverageMapping/break.c
cfe/trunk/test/CoverageMapping/continue.c
cfe/trunk/test/CoverageMapping/if.cpp
cfe/trunk/test/CoverageMapping/includehell.cpp
cfe/trunk/test/CoverageMapping/label.cpp
cfe/trunk/test/CoverageMapping/loops.cpp
cfe/trunk/test/CoverageMapping/macro-expressions.cpp
cfe/trunk/test/CoverageMapping/macros.c
cfe/trunk/test/CoverageMapping/objc.m
cfe/trunk/test/CoverageMapping/return.c
cfe/trunk/test/CoverageMapping/test.c
cfe/trunk/test/CoverageMapping/while.c

Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp?rev=319373=319372=319373=diff
==
--- cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp (original)
+++ cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp Wed Nov 29 14:25:14 2017
@@ -112,6 +112,9 @@ struct SpellingRegion {
 ColumnEnd = SM.getSpellingColumnNumber(LocEnd);
   }
 
+  SpellingRegion(SourceManager , SourceMappingRegion )
+  : SpellingRegion(SM, R.getStartLoc(), R.getEndLoc()) {}
+
   /// Check if the start and end locations appear in source order, i.e
   /// top->bottom, left->right.
   bool isInSourceOrder() const {
@@ -583,6 +586,7 @@ struct CounterCoverageMappingBuilder
   MostRecentLocation = getIncludeOrExpansionLoc(EndLoc);
 
 assert(SM.isWrittenInSameFile(Region.getStartLoc(), EndLoc));
+assert(SpellingRegion(SM, Region).isInSourceOrder());
 SourceRegions.push_back(Region);
 
 if (ParentOfDeferredRegion) {
@@ -718,9 +722,11 @@ struct CounterCoverageMappingBuilder
   SourceLocation Loc = MostRecentLocation;
   while (isNestedIn(Loc, ParentFile)) {
 SourceLocation FileStart = getStartOfFileOrMacro(Loc);
-if (StartLocs.insert(FileStart).second)
+if (StartLocs.insert(FileStart).second) {
   SourceRegions.emplace_back(*ParentCounter, FileStart,
  getEndOfFileOrMacro(Loc));
+  assert(SpellingRegion(SM, SourceRegions.back()).isInSourceOrder());
+}
 Loc = getIncludeOrExpansionLoc(Loc);
   }
 }
@@ -753,12 +759,31 @@ struct CounterCoverageMappingBuilder
 LastTerminatedRegion = {EndLoc, RegionStack.size()};
   }
 
+  /// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
+  Optional findGapAreaBetween(SourceLocation AfterLoc,
+   SourceLocation BeforeLoc) {
+// If the start and end locations of the gap are both within the same macro
+// file, the range may not be in source order.
+if (AfterLoc.isMacroID() || BeforeLoc.isMacroID())
+  return None;
+if (!SM.isWrittenInSameFile(AfterLoc, BeforeLoc))
+  return None;
+return {{AfterLoc, BeforeLoc}};
+  }
+
+  /// Find the source range after \p AfterStmt and before \p BeforeStmt.
+  Optional findGapAreaBetween(const Stmt *AfterStmt,
+   const Stmt *BeforeStmt) {
+return findGapAreaBetween(getPreciseTokenLocEnd(getEnd(AfterStmt)),
+  getStart(BeforeStmt));
+  }
+
   /// Emit a gap region between \p StartLoc and \p EndLoc with the given count.
   void fillGapAreaWithCount(SourceLocation StartLoc, SourceLocation EndLoc,
 Counter Count) {
-if (StartLoc == EndLoc || StartLoc.isMacroID() || EndLoc.isMacroID() ||
-!SM.isWrittenInSameFile(StartLoc, EndLoc))
+if (StartLoc == EndLoc)
   return;
+assert(SpellingRegion(SM, StartLoc, EndLoc).isInSourceOrder());
 handleFileExit(StartLoc);
 size_t Index = pushRegion(Count, StartLoc, EndLoc);
 getRegion().setGap(true);
@@ -914,6 +939,11 @@ struct CounterCoverageMappingBuilder
 propagateCounts(CondCount, S->getCond());
 adjustForOutOfOrderTraversal(getEnd(S));
 
+// The body count applies to the area immediately after the increment.
+auto Gap = findGapAreaBetween(S->getCond(), S->getBody());
+if (Gap)
+  fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
+
 Counter OutCount =
 addCounters(BC.BreakCount, subtractCounters(CondCount, BodyCount));
 if (OutCount != ParentCount)
@@ -968,6 +998,12 @@ struct CounterCoverageMappingBuilder
   adjustForOutOfOrderTraversal(getEnd(S));
 }
 

[PATCH] D40528: add new check to find NSError init invocation

[Yan Zhang via Phabricator via cfe-commits]

Wizard added inline comments.



Comment at: docs/clang-tidy/checks/objc-avoid-nserror-init.rst:10
+``errorWithDomain:code:userInfo:`` to create new NSError objects instead
+of ``[NSError alloc] init]``. Otherwise it will lead to a warning message
+during compilation in Xcode.

hokein wrote:
> Wizard wrote:
> > hokein wrote:
> > > What's the warning message in Xcode? I suspect whether there is a 
> > > diagnostic flag in clang already. 
> > It was discussed originally here 
> > https://buganizer.corp.google.com/issues/62445078 I think
> Thanks. Please don't include any internal links next time.
> 
> Looks like the error message 
> (https://stackoverflow.com/questions/33720042/why-does-nserror-alloc-init-in-xcode-throw-an-error)
>  is shown during runtime, instead of compilation. Could you please confirm 
> it, and update the document here? 
> 
> > [NSError init] called; this results in an invalid NSError instance. It will 
> > raise an exception in a future release. Please call 
> > errorWithDomain:code:userInfo: or initWithDomain:code:userInfo:. This 
> > message shown only once.
Oh yes it is shown during runtime. Will update the doc


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40528



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

[PATCH] D40528: add new check to find NSError init invocation

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 124818.
Wizard marked 3 inline comments as done.
Wizard added a comment.

address comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40528

Files:
  clang-tidy/objc/AvoidNserrorInitCheck.cpp
  clang-tidy/objc/AvoidNserrorInitCheck.h
  clang-tidy/objc/CMakeLists.txt
  clang-tidy/objc/ObjCTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/objc-avoid-nserror-init.rst
  test/clang-tidy/objc-avoid-nserror-init.m

Index: test/clang-tidy/objc-avoid-nserror-init.m
===
--- /dev/null
+++ test/clang-tidy/objc-avoid-nserror-init.m
@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s objc-avoid-nserror-init %t
+@interface NSError
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+@implementation foo
+- (void)bar {
+NSError *error = [[NSError alloc] init];
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use errorWithDomain:code:userInfo: or initWithDomain:code:userInfo: to create a new NSError [objc-avoid-nserror-init]
+}
+@end
Index: docs/clang-tidy/checks/objc-avoid-nserror-init.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/objc-avoid-nserror-init.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-avoid-nserror-init
+
+objc-avoid-nserror-init
+===
+
+This check will find out improper initialization of NSError objects.
+
+According to Apple developer document, we should always use factory method 
+``errorWithDomain:code:userInfo:`` to create new NSError objects instead
+of ``[NSError alloc] init]``. Otherwise it will lead to a warning message
+during runtime.
+
+The corresponding information about NSError creation: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ErrorHandlingCocoa/CreateCustomizeNSError/CreateCustomizeNSError.html
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -174,6 +174,7 @@
modernize-use-using
mpi-buffer-deref
mpi-type-mismatch
+   objc-avoid-nserror-init
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,11 @@
 Improvements to clang-tidy
 --
 
+- New `objc-avoid-nserror-init
+  `_ check
+
+  Add new check to detect the use of [NSError init].
+
 - New `objc-avoid-spinlock
   `_ check
 
Index: clang-tidy/objc/ObjCTidyModule.cpp
===
--- clang-tidy/objc/ObjCTidyModule.cpp
+++ clang-tidy/objc/ObjCTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidNSErrorInitCheck.h"
 #include "AvoidSpinlockCheck.h"
 #include "ForbiddenSubclassingCheck.h"
 #include "PropertyDeclarationCheck.h"
@@ -23,6 +24,8 @@
 class ObjCModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"objc-avoid-nserror-init");
 CheckFactories.registerCheck(
 "objc-avoid-spinlock");
 CheckFactories.registerCheck(
Index: clang-tidy/objc/CMakeLists.txt
===
--- clang-tidy/objc/CMakeLists.txt
+++ clang-tidy/objc/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyObjCModule
+  AvoidNSErrorInitCheck.cpp
   AvoidSpinlockCheck.cpp
   ForbiddenSubclassingCheck.cpp
   ObjCTidyModule.cpp
Index: clang-tidy/objc/AvoidNserrorInitCheck.h
===
--- /dev/null
+++ clang-tidy/objc/AvoidNserrorInitCheck.h
@@ -0,0 +1,36 @@
+//===--- AvoidNSErrorInitCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_AVOIDNSERRORINITCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_AVOIDNSERRORINITCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds usages of [NSSError init]. It is not the proper way of creating
+/// NSError. errorWithDomain:code:userInfo: should be used instead.
+///
+/// For the user-facing documentation see:
+/// 

[clang-tools-extra] r319369 - [Documentation] Sort Clang-tidy changes next way: new modules, new checks, renamed checks, extended checks, new check aliases.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed Nov 29 14:17:39 2017
New Revision: 319369

URL: http://llvm.org/viewvc/llvm-project?rev=319369=rev
Log:
[Documentation] Sort Clang-tidy changes next way: new modules, new checks, 
renamed checks, extended checks, new check aliases.

Sort checks in each section alphabetically.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-avoid-spinlock.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=319369=319368=319369=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Nov 29 14:17:39 2017
@@ -57,114 +57,10 @@ The improvements are...
 Improvements to clang-tidy
 --
 
-- New `objc-avoid-spinlock
-  `_ 
check
-
-  Add new check to detect the use of OSSpinlock.
-
-- The 'misc-move-const-arg' check was renamed to `performance-move-const-arg
-  
`_
-
-- The 'misc-noexcept-move-constructor' check was renamed to 
`performance-noexcept-move-constructor
-  
`_
-
-- The 'misc-move-constructor-init' check was renamed to 
`performance-move-constructor-init
-  
`_
-
-- The 'misc-inefficient-algorithm' check was renamed to 
`performance-inefficient-algorithm
-  
`_
-
-- The 'misc-virtual-near-miss' check was renamed to `bugprone-virtual-near-miss
-  
`_
-
-- The 'misc-use-after-move' check was renamed to `bugprone-use-after-move
-  
`_
-
-- The 'misc-multiple-statement-macro' check was renamed to 
`bugprone-multiple-statement-macro
-  
`_
-
-- The 'misc-move-forwarding-reference' check was renamed to 
`bugprone-move-forwarding-reference
-  
`_
-
-- The 'misc-inaccurate-erase' check was renamed to `bugprone-inaccurate-erase
-  
`_
-
-- The 'misc-forward-declaration-namespace' check was renamed to 
`bugprone-forward-declaration-namespace
-  
`_
-
-- The 'misc-fold-init-type' check was renamed to `bugprone-fold-init-type
-  
`_
-
-- The 'misc-bool-pointer-implicit-conversion' check was renamed to 
`bugprone-bool-pointer-implicit-conversion
-  
`_
-
-- The 'misc-assert-side-effect' check was renamed to 
`bugprone-assert-side-effect
-  
`_
-
-- The 'misc-dangling-handle' check was renamed to `bugprone-dangling-handle
-  
`_
-
-- The 'misc-argument-comment' check was renamed to `bugprone-argument-comment
-  
`_
-
-- The 'misc-string-constructor' check was renamed to 
`bugprone-string-constructor
-  
`_
-
-- New `fuchsia-default-arguments
-  
`_
 check
-
-  Warns if a function or method is declared or called with default arguments.
-
-- New `google-avoid-throwing-objc-exception
-  
`_
 check
-
-  Add new check to detect throwing exceptions in Objective-C code, which 
should be avoided.
-
-- New `objc-property-declaration
-  
`_
 check
-
-  Add new check for Objective-C code to ensure property
-  names follow the naming convention of Apple's programming
-  guide.
-
-- New `google-objc-global-variable-declaration
-  
`_
 check
-
-  Add new check for Objective-C code to ensure global 
-  variables follow the naming convention of 'k[A-Z].*' (for constants) 
-  or 'g[A-Z].*' (for 

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

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

Thanks for the approval Aaron.  I do not have commit access, would you mind 
submitting this on my behalf?


https://reviews.llvm.org/D40574



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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via cfe-commits]

On Wed, 29 Nov 2017, Martin Storsjö via cfe-commits wrote:


On Wed, 29 Nov 2017, Reid Kleckner wrote:


On Wed, Nov 29, 2017 at 12:21 PM, Martin Storsjö  wrote:
  On Wed, 29 Nov 2017, Martell Malone via cfe-commits wrote:
Thanks for letting me know Reid.
I’ll in work and won’t be able to access the repo
until lunch time. (~3
hours)
Feel free to revert if it is not trivial.

The easy fix might be to change to == x86_64 from !=
x86 For is Windows in
the default toolchain. That should restore the old
behavior.


  My suggestion would be to just return None for all architectures
  for the default windows (msvc) case. We didn't use to set any
  defines to indicate EH mode there before anyway, so setting it
  to None should make things behave just as before, right?


I did this slightly differently in r319363, but maybe that's silly. My
reasoning was that `clang -cc1 -fseh-exceptions -fexceptions` in the MSVC
environment should still use __CxxFrameHandler3. -fseh-exceptions indicates
what format of unwind information we should use, and we're still using the
normal SEH .xdata opcodes.


No, I think this change makes sense - I was just about to write a comment 
pointing out this spot in the code but was waiting for a compile to finish 
before posting.



Alternatively, you could view -fseh-exceptions, -fdwarf-exceptions, and
-fsjlj-exceptions as choices of EH personality function,


No, I don't think that'd make sense


FWIW, another reason I don't think that makes sense, is that making a 
GNU_CPlusPlus_SEH + MSVC combination probably would require quite a bit 
more changes as well.


When I tested the same in a build with assertions enabled, I didn't get 
the references to _Unwind_Resume as you did, but the compilation failed on 
some internal assertion. So making GNU_CPlusPlus_SEH usable with the 
microsoft C++ ABI would probably require a significant amount of more 
work, for very little value.


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

[PATCH] D40588: [OpenMP] Diagnose undeclared variables on declare target clause

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 updated this revision to Diff 124811.
kkwli0 added a comment.

The assert occurs in VarOrFuncDeclFilterCCC::ValidateCandidate when 
clang::Sema::CorrectTypo is called.


https://reviews.llvm.org/D40588

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/declare_target_messages.cpp


Index: test/OpenMP/declare_target_messages.cpp
===
--- test/OpenMP/declare_target_messages.cpp
+++ test/OpenMP/declare_target_messages.cpp
@@ -13,6 +13,10 @@
 
 #pragma omp declare target map(a) // expected-error {{unexpected 'map' clause, 
only 'to' or 'link' clauses expected}}
 
+#pragma omp declare target to(foo1) // expected-error {{use of undeclared 
identifier 'foo1'}}
+
+#pragma omp declare target link(foo2) // expected-error {{use of undeclared 
identifier 'foo2'}}
+
 void c(); // expected-warning {{declaration is not declared in any declare 
target region}}
 
 extern int b;
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -1506,7 +1506,7 @@
   explicit VarOrFuncDeclFilterCCC(Sema ) : SemaRef(S) {}
   bool ValidateCandidate(const TypoCorrection ) override {
 NamedDecl *ND = Candidate.getCorrectionDecl();
-if (isa(ND) || isa(ND)) {
+if (ND && (isa(ND) || isa(ND))) {
   return SemaRef.isDeclInScope(ND, SemaRef.getCurLexicalContext(),
SemaRef.getCurScope());
 }


Index: test/OpenMP/declare_target_messages.cpp
===
--- test/OpenMP/declare_target_messages.cpp
+++ test/OpenMP/declare_target_messages.cpp
@@ -13,6 +13,10 @@
 
 #pragma omp declare target map(a) // expected-error {{unexpected 'map' clause, only 'to' or 'link' clauses expected}}
 
+#pragma omp declare target to(foo1) // expected-error {{use of undeclared identifier 'foo1'}}
+
+#pragma omp declare target link(foo2) // expected-error {{use of undeclared identifier 'foo2'}}
+
 void c(); // expected-warning {{declaration is not declared in any declare target region}}
 
 extern int b;
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -1506,7 +1506,7 @@
   explicit VarOrFuncDeclFilterCCC(Sema ) : SemaRef(S) {}
   bool ValidateCandidate(const TypoCorrection ) override {
 NamedDecl *ND = Candidate.getCorrectionDecl();
-if (isa(ND) || isa(ND)) {
+if (ND && (isa(ND) || isa(ND))) {
   return SemaRef.isDeclInScope(ND, SemaRef.getCurLexicalContext(),
SemaRef.getCurScope());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1029
+  SourceLocation LocStart = ValSourceRange.getBegin();
+  SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 
0,
+ SourceMgr, LangOpts);

  const FileEntry *F =
  SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));
  if (!F)
return llvm::errc::no_such_file_or_directory;



Comment at: clangd/ClangdUnit.cpp:1039
+  Location L;
+  if (const FileEntry *F =
+  SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart))) {

FE can be null. How about returning a llvm::ErrorOr ?



Comment at: clangd/ClangdUnit.cpp:1242
+
+  const FileEntry *FE =
+  AST.getASTContext().getSourceManager().getFileManager().getFile(

FE can be null. How about returning a llvm::ErrorOr ?


https://reviews.llvm.org/D35894



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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via cfe-commits]

On Wed, 29 Nov 2017, Reid Kleckner wrote:


On Wed, Nov 29, 2017 at 12:21 PM, Martin Storsjö  wrote:
  On Wed, 29 Nov 2017, Martell Malone via cfe-commits wrote:
Thanks for letting me know Reid.
I’ll in work and won’t be able to access the repo
until lunch time. (~3
hours)
Feel free to revert if it is not trivial.

The easy fix might be to change to == x86_64 from !=
x86 For is Windows in
the default toolchain. That should restore the old
behavior.


  My suggestion would be to just return None for all architectures
  for the default windows (msvc) case. We didn't use to set any
  defines to indicate EH mode there before anyway, so setting it
  to None should make things behave just as before, right?


I did this slightly differently in r319363, but maybe that's silly. My
reasoning was that `clang -cc1 -fseh-exceptions -fexceptions` in the MSVC
environment should still use __CxxFrameHandler3. -fseh-exceptions indicates
what format of unwind information we should use, and we're still using the
normal SEH .xdata opcodes.


No, I think this change makes sense - I was just about to write a comment 
pointing out this spot in the code but was waiting for a compile to finish 
before posting.



Alternatively, you could view -fseh-exceptions, -fdwarf-exceptions, and
-fsjlj-exceptions as choices of EH personality function,


No, I don't think that'd make sense

in which case, my change is wrong, and we should do what you guys were 
suggesting and stop passing this from the driver. Hard to say.


My point was that for the MSVC mode, this worked fine before since this 
was the default behaviour - avoiding passing any option (i.e. having that 
function return None) should be a safe way to keep things working as it 
did before. But then it would break again if -fseh-exceptions were passed, 
which I would expect to be a no-op in such a configuration.


My remaining concern is mostly about why we still need the workaround for 
x86 in the function getting the default (returning None instead of WinEH 
for that case). But as long as this works and the rest of this change can 
settle, we can look at that later if any further change is warranted at 
all.


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

[PATCH] D40588: [OpenMP] Diagnose undeclared variables on declare target clause

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

The original case has the variables named "foo1" and foo2".  Using "foo1" or 
"foo2" causes the assert!  I update the description.


https://reviews.llvm.org/D40588



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

[PATCH] D38425: [clangd] Document highlights for clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 124810.
Nebiroth added a comment.

Added verification for llvm::Expected in onDocumentHighlight
Removed unused variable in ClangdUnit


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38425

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/documenthighlight.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -20,6 +20,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "definitionProvider": true,
 # CHECK-NEXT:  "documentFormattingProvider": true,
+# CHECK-NEXT:	   "documentHighlightProvider": true,
 # CHECK-NEXT:  "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:"firstTriggerCharacter": "}",
 # CHECK-NEXT:"moreTriggerCharacter": []
Index: test/clangd/documenthighlight.test
===
--- /dev/null
+++ test/clangd/documenthighlight.test
@@ -0,0 +1,42 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 479
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};\nstruct Foo {\nint xasd;\n};\n}\nint main() {\nint bonjour;\nbonjour = 2;\nint test1 = bonjour;\nns1::Foo bar = { xasd : 1};\nbar.xasd = 3;\nns1::MyClass* Params;\nParams->anotherOperation();\n}\n"}}}
+
+Content-Length: 156
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":17,"character":2}}}
+# Verify local variable 
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":17}}}
+# Verify struct highlight
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]}
+
+Content-Length: 157
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":21,"character":10}}}
+# Verify method highlight
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":14,"line":2},"start":{"character":7,"line":2}}},{"kind":1,"range":{"end":{"character":22,"line":6},"start":{"character":15,"line":6}}},{"kind":1,"range":{"end":{"character":12,"line":21},"start":{"character":5,"line":21}}}]}
+
+Content-Length: 156
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":18,"character":14}}}
+# Verify Read-access of a symbol (kind = 2) 
+# CHECK: {"id":1,"jsonrpc":"2.0","result":[{"kind":1,"range":{"end":{"character":11,"line":16},"start":{"character":4,"line":16}}},{"kind":3,"range":{"end":{"character":7,"line":17},"start":{"character":0,"line":17}}},{"kind":2,"range":{"end":{"character":19,"line":18},"start":{"character":12,"line":18}}}]}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}			
\ No newline at end of file
Index: clangd/ProtocolHandlers.h

[PATCH] D40621: MS ABI: Treat explicit instantiation definitions of dllimport function templates as explicit instantiation decls (PR35435)

[Reid Kleckner via Phabricator via cfe-commits]

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

Looks good, thanks!


https://reviews.llvm.org/D40621



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

r319363 - [EH] Use __CxxFrameHandler3 for C++ EH in MS environments

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Nov 29 13:35:34 2017
New Revision: 319363

URL: http://llvm.org/viewvc/llvm-project?rev=319363=rev
Log:
[EH] Use __CxxFrameHandler3 for C++ EH in MS environments

Fixes regression introduced by r319297. MSVC environments still use SEH
unwind opcodes but they should use the Microsoft C++ EH personality, not
the mingw one.

Added:
cfe/trunk/test/CodeGenCXX/ms-eh-personality.cpp
Modified:
cfe/trunk/lib/CodeGen/CGException.cpp

Modified: cfe/trunk/lib/CodeGen/CGException.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=319363=319362=319363=diff
==
--- cfe/trunk/lib/CodeGen/CGException.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGException.cpp Wed Nov 29 13:35:34 2017
@@ -205,12 +205,9 @@ const EHPersonality ::get(
   if (T.isWindowsMSVCEnvironment() && !L.ObjC1) {
 if (L.SjLjExceptions)
   return EHPersonality::GNU_CPlusPlus_SJLJ;
-if (L.SEHExceptions)
-  return EHPersonality::GNU_CPlusPlus_SEH;
 if (L.DWARFExceptions)
   return EHPersonality::GNU_CPlusPlus;
-else
-  return EHPersonality::MSVC_CxxFrameHandler3;
+return EHPersonality::MSVC_CxxFrameHandler3;
   }
 
   if (L.CPlusPlus && L.ObjC1)

Added: cfe/trunk/test/CodeGenCXX/ms-eh-personality.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-eh-personality.cpp?rev=319363=auto
==
--- cfe/trunk/test/CodeGenCXX/ms-eh-personality.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ms-eh-personality.cpp Wed Nov 29 13:35:34 2017
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fexceptions -fcxx-exceptions 
%s -emit-llvm -o - | FileCheck %s --check-prefix=MSVC
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fexceptions -fcxx-exceptions 
-fsjlj-exceptions %s -emit-llvm -o - | FileCheck %s --check-prefix=SJLJ
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fexceptions -fcxx-exceptions 
-fseh-exceptions %s -emit-llvm -o - | FileCheck %s --check-prefix=MSVC
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fexceptions -fcxx-exceptions 
-fdwarf-exceptions %s -emit-llvm -o - | FileCheck %s --check-prefix=DWARF
+
+// MSVC: define void @f(){{.*}}@__CxxFrameHandler3
+// SJLJ: define void @f(){{.*}}@__gxx_personality_sj0
+// DWARF: define void @f(){{.*}}@__gxx_personality_v0
+
+struct Cleanup {
+  Cleanup();
+  ~Cleanup();
+  int x = 0;
+};
+
+void g();
+extern "C" void f() {
+  Cleanup c;
+  g();
+}


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

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D40563#939555, @arphaman wrote:

> Could you please add a test?


Any tip on how this should be tested? I couldn't find any existing unit test 
for either SemaCodeComplete or code completion context (under 
`clang/unittests`). It might be possible to set up a unit test framework for 
code completion, but I'm not sure if it's worth to do so since this change 
doesn't change completion behavior.


https://reviews.llvm.org/D40563



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

r319362 - [OPENMP] Allow only loop control variables in distribute simd

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Nov 29 13:31:48 2017
New Revision: 319362

URL: http://llvm.org/viewvc/llvm-project?rev=319362=rev
Log:
[OPENMP] Allow only loop control variables in distribute simd
directives.

According to the OpenMP standard, only loop control variables can be
used in linear clauses of distribute-based simd directives.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_ast_print.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_misc_messages.c
cfe/trunk/test/OpenMP/distribute_simd_ast_print.cpp
cfe/trunk/test/OpenMP/distribute_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/distribute_simd_misc_messages.c

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_ast_print.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_ast_print.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_ast_print.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_ast_print.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_linear_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=319362=319361=319362=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 29 13:31:48 
2017
@@ -8957,6 +8957,8 @@ def err_omp_reduction_with_nogroup : Err
   "'reduction' clause cannot be used with 'nogroup' clause">;
 def err_omp_reduction_vla_unsupported : Error<
   "cannot generate code for reduction on %select{|array section, which 
requires a }0variable length array">;
+def err_omp_linear_distribute_var_non_loop_iteration : Error<
+  "only loop iteration variables are allowed in 'linear' clause in distribute 
directives">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=319362=319361=319362=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Nov 29 13:31:48 2017
@@ -6808,17 +6808,6 @@ StmtResult Sema::ActOnOpenMPDistributePa
   assert((CurContext->isDependentContext() || B.builtAll()) &&
  "omp for loop exprs were not built");
 
-  if (!CurContext->isDependentContext()) {
-// Finalize the clauses that need pre-built expressions for CodeGen.
-for (auto C : Clauses) {
-  if (auto *LC = dyn_cast(C))
-if (FinishOpenMPLinearClause(*LC, cast(B.IterationVarRef),
- B.NumIterations, *this, CurScope,
- DSAStack))
-  return StmtError();
-}
-  }
-
   getCurFunction()->setHasBranchProtectedScope();
   return OMPDistributeParallelForDirective::Create(
   Context, StartLoc, EndLoc, NestedLoopCount, Clauses, AStmt, B,
@@ -7223,17 +7212,6 @@ StmtResult Sema::ActOnOpenMPTeamsDistrib
   assert((CurContext->isDependentContext() || B.builtAll()) &&
  "omp for loop exprs were not built");
 
-  if (!CurContext->isDependentContext()) {
-// Finalize the clauses that need pre-built expressions for CodeGen.
-for (auto C : Clauses) {
-  if (auto *LC = dyn_cast(C))
-if (FinishOpenMPLinearClause(*LC, cast(B.IterationVarRef),
- B.NumIterations, *this, CurScope,
- DSAStack))
-  return StmtError();
-}
-  }
-
   getCurFunction()->setHasBranchProtectedScope();
   return OMPTeamsDistributeParallelForDirective::Create(
   Context, StartLoc, EndLoc, NestedLoopCount, Clauses, AStmt, B,
@@ -7334,17 +7312,6 @@ StmtResult Sema::ActOnOpenMPTargetTeamsD
   assert((CurContext->isDependentContext() || B.builtAll()) &&
  "omp target teams distribute parallel for loop exprs were not built");
 
-  if (!CurContext->isDependentContext()) {
-// Finalize the clauses that need pre-built expressions for CodeGen.
-for (auto C : Clauses) {
-  if (auto *LC = dyn_cast(C))
-if (FinishOpenMPLinearClause(*LC, cast(B.IterationVarRef),
- B.NumIterations, *this, CurScope,
- DSAStack))
-  return StmtError();
-}
-  }
-
   getCurFunction()->setHasBranchProtectedScope();
   return 

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[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.

Thanks, LGTM!


https://reviews.llvm.org/D40574



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

[PATCH] D40621: MS ABI: Treat explicit instantiation definitions of dllimport function templates as explicit instantiation decls (PR35435)

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.

This matches MSVC's behaviour, and we already do it for class templates since 
r270897.


https://reviews.llvm.org/D40621

Files:
  lib/Sema/SemaTemplate.cpp
  test/CodeGenCXX/dllimport.cpp


Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -579,7 +579,7 @@
 // MSC-DAG: declare dllimport void 
@"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GNU-DAG: declare dllimport void @_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GNU-DAG: define weak_odr void 
@_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
-// MO1-DAG: define available_externally dllimport void 
@"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
+// MO1-DAG: declare dllimport void 
@"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
 // MO1-DAG: define available_externally dllimport void 
@"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GO1-DAG: define available_externally dllimport void 
@_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GO1-DAG: define weak_odr void 
@_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
@@ -609,6 +609,15 @@
 template<> __declspec(dllimport) inline void 
funcTmpl() {}
 USE(funcTmpl)
 
+#ifdef MSABI
+namespace pr35435 {
+struct X;
+template  struct __declspec(dllimport) S {
+  void foo(T *t) { t->problem(); }
+};
+template void S::foo(X*); // Cannot be instantiated because X is 
incomplete; dllimport means it's treated as an instantiation decl.
+}
+#endif
 
 
 
//===--===//
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -9239,10 +9239,18 @@
   return (Decl*) nullptr;
   }
 
-  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
   if (Attr)
 ProcessDeclAttributeList(S, Specialization, Attr);
 
+  // In MSVC mode, dllimported explicit instantiation definitions are treated 
as
+  // instantiation declarations.
+  if (TSK == TSK_ExplicitInstantiationDefinition &&
+  Specialization->hasAttr() &&
+  Context.getTargetInfo().getCXXABI().isMicrosoft())
+TSK = TSK_ExplicitInstantiationDeclaration;
+
+  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
+
   if (Specialization->isDefined()) {
 // Let the ASTConsumer know that this function has been explicitly
 // instantiated now, and its linkage might have changed.


Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -579,7 +579,7 @@
 // MSC-DAG: declare dllimport void @"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GNU-DAG: declare dllimport void @_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GNU-DAG: define weak_odr void @_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
-// MO1-DAG: define available_externally dllimport void @"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
+// MO1-DAG: declare dllimport void @"\01??$funcTmpl@UExplicitInst_ImportedYAXXZ"()
 // MO1-DAG: define available_externally dllimport void @"\01??$inlineFuncTmpl@UExplicitInst_ImportedYAXXZ"()
 // GO1-DAG: define available_externally dllimport void @_Z8funcTmplI21ExplicitInst_ImportedEvv()
 // GO1-DAG: define weak_odr void @_Z14inlineFuncTmplI21ExplicitInst_ImportedEvv()
@@ -609,6 +609,15 @@
 template<> __declspec(dllimport) inline void funcTmpl() {}
 USE(funcTmpl)
 
+#ifdef MSABI
+namespace pr35435 {
+struct X;
+template  struct __declspec(dllimport) S {
+  void foo(T *t) { t->problem(); }
+};
+template void S::foo(X*); // Cannot be instantiated because X is incomplete; dllimport means it's treated as an instantiation decl.
+}
+#endif
 
 
 //===--===//
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -9239,10 +9239,18 @@
   return (Decl*) nullptr;
   }
 
-  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
   if (Attr)
 ProcessDeclAttributeList(S, Specialization, Attr);
 
+  // In MSVC mode, dllimported explicit instantiation definitions are treated as
+  // instantiation declarations.
+  if (TSK == TSK_ExplicitInstantiationDefinition &&
+  Specialization->hasAttr() &&
+  Context.getTargetInfo().getCXXABI().isMicrosoft())
+TSK = TSK_ExplicitInstantiationDeclaration;
+
+  Specialization->setTemplateSpecializationKind(TSK, D.getIdentifierLoc());
+
   if (Specialization->isDefined()) {
 // Let the ASTConsumer know that this function has been explicitly
 // instantiated now, and its linkage might have changed.
___
cfe-commits mailing list

[PATCH] D40611: Add has definition AST matcher

[Aaron Ballman via Phabricator via cfe-commits]

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

Commit in r319360


https://reviews.llvm.org/D40611



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

r319360 - Add the hasDefinition() AST matcher to match class declarations that also have a definition.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Nov 29 13:21:51 2017
New Revision: 319360

URL: http://llvm.org/viewvc/llvm-project?rev=319360=rev
Log:
Add the hasDefinition() AST matcher to match class declarations that also have 
a definition.

Patch by Julie Hockett.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=319360=319359=319360=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Nov 29 13:21:51 2017
@@ -2307,6 +2307,15 @@ Usable as: Matcherhttp://cl
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclhasDefinition
+Matches a class 
declaration that is defined.
+
+Example matches x (matcher = cxxRecordDecl(hasDefinition()))
+class x {};
+class y;
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclisDerivedFromstd::string 
BaseName
 Overloaded method as 
shortcut for isDerivedFrom(hasName(...)).
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=319360=319359=319360=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Nov 29 13:21:51 2017
@@ -5853,6 +5853,17 @@ AST_MATCHER_P(CXXNewExpr, hasArraySize,
 InnerMatcher.matches(*Node.getArraySize(), Finder, Builder);
 }
 
+/// \brief Matches a class declaration that is defined.
+///
+/// Example matches x (matcher = cxxRecordDecl(hasDefinition()))
+/// \code
+/// class x {};
+/// class y;
+/// \endcode
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  return Node.hasDefinition();
+}
+
 } // namespace ast_matchers
 } // namespace clang
 

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=319360=319359=319360=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Nov 29 13:21:51 2017
@@ -250,6 +250,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasDeclContext);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
+  REGISTER_MATCHER(hasDefinition);
   REGISTER_MATCHER(hasDescendant);
   REGISTER_MATCHER(hasDestinationType);
   REGISTER_MATCHER(hasDynamicExceptionSpec);

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=319360=319359=319360=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Wed Nov 29 
13:21:51 2017
@@ -2008,5 +2008,26 @@ TEST(HasArraySize, Basic) {
   cxxNewExpr(hasArraySize(integerLiteral(equals(10));
 }
 
+TEST(HasDefinition, MatchesStructDefinition) {
+  EXPECT_TRUE(matches("struct x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("struct x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesClassDefinition) {
+  EXPECT_TRUE(matches("class x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("class x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesUnionDefinition) {
+  EXPECT_TRUE(matches("union x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("union x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
 } // namespace ast_matchers
 } // namespace clang


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

[PATCH] D40611: Add has definition AST matcher

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

That'd  be great -- thank you! I did request commit rights, so hopefully I'll 
be able to do it myself next time.


https://reviews.llvm.org/D40611



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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.h:320
+
+StringRef getDataBufferFromSourceRange(ParsedAST , SourceRange SR,
+   Location L);

I think this can be only in the source file, in a an anonymous namespace.


https://reviews.llvm.org/D35894



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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC319357: [SourceLocations] Use stronger sort predicate to 
remove non-deterministic… (authored by mgrang).

Repository:
  rC Clang

https://reviews.llvm.org/D40618

Files:
  tools/libclang/CIndex.cpp


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319357: [SourceLocations] Use stronger sort predicate to 
remove non-deterministic… (authored by mgrang).

Changed prior to commit:
  https://reviews.llvm.org/D40618?vs=124798=124804#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40618

Files:
  cfe/trunk/tools/libclang/CIndex.cpp


Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319357 - [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Wed Nov 29 12:55:13 2017
New Revision: 319357

URL: http://llvm.org/viewvc/llvm-project?rev=319357=rev
Log:
[SourceLocations] Use stronger sort predicate to remove non-deterministic 
ordering

Summary:
This fixes the following failure uncovered by D39245:
  Clang :: Index/getcursor-preamble.m

Reviewers: gbenyei, akyrtzi, bkramer, arphaman

Reviewed By: arphaman

Subscribers: arphaman, cfe-commits

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

Modified:
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=319357=319356=319357=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Wed Nov 29 12:55:13 2017
@@ -1025,8 +1025,9 @@ bool CursorVisitor::VisitObjCContainerDe
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


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

[PATCH] D40611: Add has definition AST matcher

[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.

Aside from a minor commenting nit, LGTM! Do you need me to commit this on your 
behalf? (If so, I can take care of the commenting fix.)




Comment at: include/clang/ASTMatchers/ASTMatchers.h:5856
 
+/// \brief Matches a declaration that is defined.
+///

Might want to mention that this is specific to classes and not other things, 
like functions.


https://reviews.llvm.org/D40611



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

[PATCH] D40256: [ARM] disable FPU features when using soft floating point.

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D40256



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

[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Could you please add a test?


https://reviews.llvm.org/D40563



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

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/ClangdUnit.cpp:1135
+RawComment *RC =
+AST.getASTContext().getRawCommentForDeclNoCache(LocationDecl);
+if (RC) {

Not sure why but I don't get the comments when I hover on "push_back" but I do 
get it on many other methods. This can be investigated separately I think. It 
could be a bug in getRawCommentForDeclNoCache


https://reviews.llvm.org/D35894



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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Alex Lorenz via Phabricator via cfe-commits]

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

Thanks, LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D40618



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

[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

You probably just need to pass a "-triple" flag so we don't use Windows 
mangling or something like that.


Repository:
  rL LLVM

https://reviews.llvm.org/D39627



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

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

By many years of precedent, the "frem" instruction is supposed to match the C 
fmod(), as opposed to something else like the C99 remainder(); probably worth 
clarifying in LangRef.


https://reviews.llvm.org/D40594



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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martin Storsjö via cfe-commits]

On Wed, 29 Nov 2017, Martell Malone via cfe-commits wrote:


Thanks for letting me know Reid.
I’ll in work and won’t be able to access the repo until lunch time. (~3
hours)
Feel free to revert if it is not trivial.

The easy fix might be to change to == x86_64 from != x86 For is Windows in
the default toolchain. That should restore the old behavior.


My suggestion would be to just return None for all architectures for the 
default windows (msvc) case. We didn't use to set any defines to indicate 
EH mode there before anyway, so setting it to None should make things 
behave just as before, right?


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

[PATCH] D40508: Replace long type names in IR with hashes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D40508#938854, @sepavloff wrote:

> In https://reviews.llvm.org/D40508#938686, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D40508#938675, @sepavloff wrote:
> >
> > > In https://reviews.llvm.org/D40508#938040, @rjmccall wrote:
> > >
> > > > My skepticism about the raw_ostream is not about the design of having a 
> > > > custom raw_ostream subclass, it's that that subclass could conceivably 
> > > > be re-used by some other client.  It feels like it belongs as an 
> > > > internal hack in Clang absent some real evidence that someone else 
> > > > would use it.
> > >
> > >
> > > This class can be helpful in various cases where string identifier must 
> > > persistently identify an object and memory consumption must be low. It 
> > > may be:
> > >
> > > - If we introduce an option that allows a user to specify how many 
> > > symbols of full type name are kept in abbreviated name, then `llvm-link` 
> > > may see types named as `foo` and 
> > > `2544896211ff669ed44dccd265f4c9163b340190`, if they come from modules 
> > > compiled with different options. To find out that these are the same 
> > > type, it must have access to the same machinery.
> >
> >
> > The LLVM linking model does not actually depend on struct type names 
> > matching.  My understanding is that, at best, it uses that as a heuristic 
> > for deciding whether to make an effort to unify two types, but it's not 
> > something that's ultimately supposed to impact IR semantics.
>
>
> It is mainly true with an exception, when `llvm-link` resolves opaque types 
> it relies on type name only. And this behavior creates the issue that 
> https://reviews.llvm.org/D40567 tries to resolve.


It is not clear from that report what the actual problem is.  Two incomplete 
types get merged by the linker?  So what?

>> If we needed IR type names to match reliably, we would use a mangled name, 
>> not a pretty-print.
> 
> There is no requirement for IR type name to be an identifier, so pretty-print 
> fits the need of type identification.

Not really; pretty-printing drops a lot of information that is pertinent in a 
stable identifier, like scopes and so on, and makes arbitrary decisions about 
other things, like where to insert spaces, namespace qualifiers, etc.


https://reviews.llvm.org/D40508



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

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.



Comment at: tools/libclang/CIndex.cpp:1028
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :

Also removed the assert from here since isBeforeInTranslationUnit already has 
the same assert.


Repository:
  rC Clang

https://reviews.llvm.org/D40618



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

Re: [llvm-dev] LLVM buildmaster is back to work now but two builders remain OFF

[Greg Bedwell via cfe-commits]

Hopefully these two should be good to re-enable as of r319330.

-Greg

On 29 November 2017 at 16:23, Greg Bedwell  wrote:

> > Two slaves remain off for now as they produce a lot of warnings and
> their log files are way too big.
>
> I think https://reviews.llvm.org/D40603 will fix this issue.
>
> -Greg
>
>
> On 28 November 2017 at 22:13, Galina Kistanova via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
>
>> Hello everyone,
>>
>> LLVM buildmaster is back to work.
>>
>> Two slaves remain off for now as they produce a lot of warnings and their
>> log files are way too big.
>>
>> The next builds have full logs available. Hope this helps to fix this
>> issue:
>>
>> http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/8716
>> http://lab.llvm.org:8011/builders/clang-with-thin-lto-windows/builds/2645
>>
>> Thanks
>>
>> Galina
>>
>> ___
>> LLVM Developers mailing list
>> llvm-...@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40618: [SourceLocations] Use stronger sort predicate to remove non-deterministic ordering

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.

This fixes the following failure uncovered by https://reviews.llvm.org/D39245:

  Clang :: Index/getcursor-preamble.m


Repository:
  rC Clang

https://reviews.llvm.org/D40618

Files:
  tools/libclang/CIndex.cpp


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1025,8 +1025,9 @@
 [](Decl *A, Decl *B) {
 SourceLocation L_A = A->getLocStart();
 SourceLocation L_B = B->getLocStart();
-assert(L_A.isValid() && L_B.isValid());
-return SM.isBeforeInTranslationUnit(L_A, L_B);
+return L_A != L_B ?
+   SM.isBeforeInTranslationUnit(L_A, L_B) :
+   SM.isBeforeInTranslationUnit(A->getLocEnd(), B->getLocEnd());
   });
 
   // Now visit the decls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[David Blaikie via Phabricator via cfe-commits]

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

Not totally clear to me why this feature is desirable, but assume you've got 
use cases/reasons :)




Comment at: clang/lib/Driver/XRayArgs.cpp:32-33
 constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
+constexpr char XRayAlwaysEmitCustomEventsOption[] =
+"-fxray-always-emit-customevents";
 } // namespace

Not clear to me there's a benefit to having these defined as constants versus 
using the literal directly - other parts of the driver use literals directly & 
there's are mostly used just once?


https://reviews.llvm.org/D40601



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

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

[Kostya Serebryany via Phabricator via cfe-commits]

kcc updated this revision to Diff 124797.
kcc added a comment.

rephrase the sources of asan overhead


Repository:
  rC Clang

https://reviews.llvm.org/D40568

Files:
  docs/TaggedAddressSanitizerDesign.rst

Index: docs/TaggedAddressSanitizerDesign.rst
===
--- /dev/null
+++ docs/TaggedAddressSanitizerDesign.rst
@@ -0,0 +1,125 @@
+===
+TaggedAddressSanitizer Design Documentation
+===
+
+This page is a preliminary design document for
+a **hardware-assisted memory safety** (HWAMS) tool,
+similar to :doc:`AddressSanitizer`.
+
+The name **TaggedAddressSanitizer** or TASAN
+(alternative: **HardwareAssistedAddressSanitizer** or HWASAN)
+and the rest of the document,
+are *early draft*, suggestions are welcome.
+
+
+Introduction
+
+
+:doc:`AddressSanitizer`
+tags every 8 bytes of the application memory with a 1 byte tag (using *shadow memory*),
+uses *redzones* to find buffer-overflows and
+*quarantine* to find use-after-free.
+The redzones, the quarantine, and, to a less extent, the shadow, are the
+sources of AddressSanitizer's memory overhead.
+See the `AddressSanitizer paper`_ for details.
+
+AArch64 has the `Address Tagging`_, a hardware feature that allows
+software to use 8 most significant bits of a 64-bit pointer as
+a tag. TaggedAddressSanitizer uses `Address Tagging`_
+to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
+but with smaller memory overhead and slightly different (mostly better)
+accuracy guarantees.
+
+Algorithm
+=
+* Every heap/stack/global memory object is forcibly aligned by `N` bytes
+  (`N` is e.g. 16 or 64)
+* For every such object a random `K`-bit tag `T` is chosen (`K` is e.g. 4 or 8)
+* The pointer to the object is tagged with `T`.
+* The memory for the object is also tagged with `T`
+  (using a `N=>1` shadow memory)
+* Every load and store is instrumented to read the memory tag and compare it
+  with the pointer tag, exception is raised on tag mismatch.
+
+Instrumentation
+===
+
+Memory Accesses
+---
+All memory accesses are prefixed with a call to a run-time function.
+The function encodes the type and the size of access in its name;
+it receives the address as a parameter, e.g. `__hwams_load4(void *ptr)`;
+it loads the memory tag, compares it with the
+pointer tag, and executes `__builtin_trap` (or calls `__hwams_error_load4(void *ptr)`) on mismatch.
+
+It's possible to inline this callback too.
+
+Heap
+
+
+Tagging the heap memory/pointers is done by `malloc`.
+This can be based on any malloc that forces all objects to be N-aligned.
+
+Stack
+-
+
+Special compiler instrumentation is required to align the local variables
+by N, tag the memory and the pointers.
+Stack instrumentation is expected to be a major source of overhead,
+but could be optional.
+TODO: details.
+
+Globals
+---
+
+TODO: details.
+
+Error reporting
+---
+
+Errors are generated by `__builtin_trap` and are handled by a signal handler.
+
+
+Comparison with AddressSanitizer
+
+
+TaggedAddressSanitizer:
+  * Is less portable than :doc:`AddressSanitizer`
+as it relies on hardware `Address Tagging`_ (AArch64).
+Address Tagging can be emulated with compiler instrumentation,
+but it will require the instrumentation to remove the tags before
+any load or store, which is infeasible in any realistic environment
+that contains non-instrumented code.
+  * May have compatibility problems if the target code uses higher
+pointer bits for other purposes.
+  * May require changes in the OS kernels (e.g. Linux seems to dislike
+tagged pointers passed from address space).
+  * Does not require redzones to detect buffer overflows,
+but the buffer overflow detection is probabilistic, with roughly
+`(2**K-1)/(2**K)` probability of catching a bug.
+  * Does not require quarantine to detect heap-use-after-free,
+or stack-use-after-return.
+The detection is similarly probabilistic.
+
+The memory overhead of TaggedAddressSanitizer is expected to be much smaller
+than that of AddressSanitizer:
+`1/N` extra memory for the shadow
+and some overhead due to `N`-aligning all objects.
+
+
+Related Work
+
+* `SPARC ADI`_ implements a similar tool mostly in hardware.
+* `Effective and Efficient Memory Protection Using Dynamic Tainting`_ discusses
+  similar approaches ("lock & key").
+* `Watchdog`_ discussed a heavier, but still somewhat similar
+  "lock & key" approach.
+* *TODO: add more "related work" links. Suggestions are welcome.*
+
+
+.. _Watchdog: http://www.cis.upenn.edu/acg/papers/isca12_watchdog.pdf
+.. _Effective and Efficient Memory Protection Using Dynamic Tainting: https://www.cc.gatech.edu/~orso/papers/clause.doudalis.orso.prvulovic.pdf
+.. _SPARC ADI: 

[PATCH] D40527: [libclang] Record parsing invocation to a temporary file when requested by client

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 124788.
arphaman added a comment.

- Capture OS only.
- Guard toolchain path accessor with a mutex.


Repository:
  rC Clang

https://reviews.llvm.org/D40527

Files:
  include/clang-c/Index.h
  test/Index/record-parsing-invocation.c
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/CIndexer.cpp
  tools/libclang/CIndexer.h
  tools/libclang/libclang.exports

Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -2,6 +2,7 @@
 clang_CXCursorSet_insert
 clang_CXIndex_getGlobalOptions
 clang_CXIndex_setGlobalOptions
+clang_CXIndex_setInvocationEmissionPathOption
 clang_CXXConstructor_isConvertingConstructor
 clang_CXXConstructor_isCopyConstructor
 clang_CXXConstructor_isDefaultConstructor
Index: tools/libclang/CIndexer.h
===
--- tools/libclang/CIndexer.h
+++ tools/libclang/CIndexer.h
@@ -18,6 +18,7 @@
 #include "clang-c/Index.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Mutex.h"
 #include 
 
 namespace llvm {
@@ -40,6 +41,11 @@
   std::string ResourcesPath;
   std::shared_ptr PCHContainerOps;
 
+  std::string ToolchainPath;
+  llvm::sys::SmartMutex Mutex;
+
+  std::string InvocationEmissionPath;
+
 public:
   CIndexer(std::shared_ptr PCHContainerOps =
std::make_shared())
@@ -71,6 +77,29 @@
 
   /// \brief Get the path of the clang resource files.
   const std::string ();
+
+  StringRef getClangToolchainPath();
+
+  void setInvocationEmissionPath(StringRef Str) {
+InvocationEmissionPath = Str;
+  }
+
+  StringRef getInvocationEmissionPath() const { return InvocationEmissionPath; }
+};
+
+/// Logs information about a particular libclang operation like parsing to
+/// a new file in the invocation emission path.
+class LibclangInvocationReporter {
+public:
+  enum class OperationKind { ParseOperation, CompletionOperation };
+
+  LibclangInvocationReporter(CIndexer , OperationKind Op,
+ unsigned ParseOptions,
+ llvm::ArrayRef Args);
+  ~LibclangInvocationReporter();
+
+private:
+  std::string File;
 };
 
   /// \brief Return the current size to request for "safety".
Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -14,8 +14,10 @@
 #include "CIndexer.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Version.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/MutexGuard.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include 
@@ -76,3 +78,59 @@
   ResourcesPath = LibClangPath.str();
   return ResourcesPath;
 }
+
+StringRef CIndexer::getClangToolchainPath() {
+  // The toolchain path might be requested by multiple threads from
+  // LibclangInvocationReporter.
+  llvm::MutexGuard Guard(Mutex);
+  if (!ToolchainPath.empty())
+return ToolchainPath;
+  StringRef ResourcePath = getClangResourcesPath();
+  ToolchainPath = llvm::sys::path::parent_path(
+  llvm::sys::path::parent_path(llvm::sys::path::parent_path(ResourcePath)));
+  return ToolchainPath;
+}
+
+LibclangInvocationReporter::LibclangInvocationReporter(
+CIndexer , OperationKind Op, unsigned ParseOptions,
+llvm::ArrayRef Args) {
+  StringRef Path = Idx.getInvocationEmissionPath();
+  if (Path.empty())
+return;
+
+  // Create a temporary file for the invocation log.
+  SmallString<256> TempPath;
+  TempPath = Path;
+  llvm::sys::path::append(TempPath, "libclang-");
+  int FD;
+  if (llvm::sys::fs::createUniqueFile(TempPath, FD, TempPath))
+return;
+  File = std::string(TempPath.begin(), TempPath.end());
+  llvm::raw_fd_ostream OS(FD, /*ShouldClose=*/true);
+
+  // Write out the information about the invocation to it.
+  auto WriteStringKey = [](StringRef Key, StringRef Value) {
+OS << R"(")" << Key << R"(":")";
+OS << Value << '"';
+  };
+  OS << '{';
+  WriteStringKey("toolchain", Idx.getClangToolchainPath());
+  OS << ',';
+  WriteStringKey("libclang.operation",
+ Op == OperationKind::ParseOperation ? "parse" : "complete");
+  OS << ',';
+  OS << R"("libclang.opts":)" << ParseOptions;
+  OS << ',';
+  OS << R"("args":[)";
+  for (const auto  : llvm::enumerate(Args)) {
+if (I.index())
+  OS << ',';
+OS << '"' << I.value() << '"';
+  }
+  OS << "]}";
+}
+
+LibclangInvocationReporter::~LibclangInvocationReporter() {
+  if (!File.empty())
+llvm::sys::fs::remove(File);
+}
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3254,6 +3254,12 @@
   return 0;
 }

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

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



Comment at: test/Sema/error-type-safety.cpp:3-4
+
+static const int test_void
+  __attribute__((type_tag_for_datatype(test, void))) = 0;
+

aaron.ballman wrote:
> Is this declaration necessary?
Hi Aaron, thanks for the input.  Yes, defining the tag is necessary in order to 
test the argument index.  If the type tag is not satisfied, the code path 
checking argument tag index will never be reached.  My update will test the 
bounds and is a little more comprehensive.


https://reviews.llvm.org/D40574



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

[PATCH] D40569: Use default IR alignment for cleanup.dest.slot.

[Jacob Young via Phabricator via cfe-commits]

jacobly added a comment.

Could someone commit this for me? Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D40569



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 124783.
mattd added a comment.

- Removed the new lines.
- Added white space between the data type and pointer character.
- Updated the test to check the exact bounds, and over-bounds cases
- Combined the diagnostic messages into one via 'select'




https://reviews.llvm.org/D40574

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  test/Sema/error-type-safety.cpp

Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define INT_TAG 42
+
+static const int test_in
+  __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
+
+// Argument index: 1, Type tag index: 2
+void test_bounds_index(...)
+  __attribute__((argument_with_type_tag(test, 1, 2)));
+
+// Argument index: 3, Type tag index: 1
+void test_bounds_arg_index(...)
+  __attribute__((argument_with_type_tag(test, 3, 1)));
+
+void test_bounds()
+{
+  // Test the boundary edges (ensure no off-by-one) with argument indexing.
+  test_bounds_index(1, INT_TAG);
+
+  test_bounds_index(1); // expected-error {{type tag index 2 is greater than the number of arguments specified}}
+  test_bounds_arg_index(INT_TAG, 1); // expected-error {{argument index 3 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
 // Type safety checking.
 if (FDecl) {
   for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
 }
   }
 
@@ -12329,10 +12329,18 @@
 }
 
 void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
   const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
   bool IsPointerAttr = Attr->getIsPointer();
 
+  // Retrieve the argument representing the 'type_tag'.
+  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 0 << Attr->getTypeTagIdx() + 1;
+return;
+  }
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
   bool FoundWrongKind;
   TypeTagData TypeInfo;
@@ -12346,6 +12354,13 @@
 return;
   }
 
+  // Retrieve the argument representing the 'arg_idx'.
+  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 1 << Attr->getArgumentIdx() + 1;
+return;
+  }
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
   if (IsPointerAttr) {
 // Skip implicit cast of pointer to `void *' (as a function argument).
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
   /// \brief Peform checks on a call of a function with argument_with_type_tag
   /// or pointer_with_type_tag attributes.
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
 
   /// \brief Check if we are taking the address of a packed field
   /// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,8 @@
   "'type_tag_for_datatype' attribute requires the initializer to be "
   "an %select{integer|integral}0 constant expression "
   "that can be represented by a 64 bit integer">;
+def err_tag_index_out_of_range : Error<
+  "%select{type tag|argument}0 index %1 is greater than the number of arguments specified">;
 def warn_type_tag_for_datatype_wrong_kind : Warning<
   "this type tag was not designed to be used with this function">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

lebedev.ri wrote:
> xazax.hun wrote:
> > lebedev.ri wrote:
> > > dcoughlin wrote:
> > > > "Unary operator" is probably not something we should expect users to 
> > > > know about. How about just "The expression is an uninitialized value. 
> > > > The computed value will also be garbage."
> > > > 
> > > Yep, i did not like my wording either :)
> > A value being undefined does nt mean uninitialized. E.g.: the result of a 
> > bad shift operation might be UndefVal as well.
> > Aren't these diagnostics misleading in those cases? Or we do not care about 
> > those?
> I think this is for @dcoughlin to answer.
> But shift operation is a binary operator, so this diff should not change that.
I was wondering about examples like:
```
int x = 1 << -1;
++x;
```

In this particular case, it will not issue the misleading error message. The 
shift that results in an undefined value will generate an error node, so we 
will not warn for the pre increment.

But in general I am a bit uncomfortable about the assumption of this check: if 
a value is undefined the reason is that it is being uninitialized. 

Note that, of course this problem is not specific to this patch but a general 
question about the checker. 


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

r319336 - [clang-cl] Alias /wd4018 to -Wno-sign-compare

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Nov 29 10:45:03 2017
New Revision: 319336

URL: http://llvm.org/viewvc/llvm-project?rev=319336=rev
Log:
[clang-cl] Alias /wd4018 to -Wno-sign-compare

This warning is known to be noisy and projects frequently disable it. In
particular, this should make building isl as bundled in polly with
clang-cl a lot quieter.

Modified:
cfe/trunk/include/clang/Driver/CLCompatOptions.td

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=319336=319335=319336=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Nov 29 10:45:03 2017
@@ -154,6 +154,8 @@ def _SLASH_WX_ : CLFlag<"WX-">, HelpText
 def _SLASH_w_flag : CLFlag<"w">, HelpText<"Disable all warnings">, Alias;
 def _SLASH_wd4005 : CLFlag<"wd4005">, Alias,
   AliasArgs<["no-macro-redefined"]>;
+def _SLASH_wd4018 : CLFlag<"wd4018">, Alias,
+  AliasArgs<["no-sign-compare"]>;
 def _SLASH_wd4100 : CLFlag<"wd4100">, Alias,
   AliasArgs<["no-unused-parameter"]>;
 def _SLASH_wd4910 : CLFlag<"wd4910">, Alias,


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

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Martell Malone via cfe-commits]

Thanks for letting me know Reid.
I’ll in work and won’t be able to access the repo until lunch time. (~3
hours)
Feel free to revert if it is not trivial.

The easy fix might be to change to == x86_64 from != x86 For is Windows in
the default toolchain. That should restore the old behavior.

I’m curious, is this in clang-cl mode?


On Wed 29 Nov 2017 at 10:12, Reid Kleckner  wrote:

> I see a bunch of link errors that suggest we're using the wrong EH
> personality in MSVC mode now:
>
> FAILED: win_clang_nacl_win64/swiftshader/libEGL.dll
> win_clang_nacl_win64/swiftshader/libEGL.dll.lib
> win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
> C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe
> ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64
> False link.exe /nologo
> /IMPLIB:win_clang_nacl_win64/swiftshader/libEGL.dll.lib /DLL
> /OUT:win_clang_nacl_win64/swiftshader/libEGL.dll
> /PDB:win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
> @win_clang_nacl_win64/swiftshader/libEGL.dll.rsp
> Config.obj : error LNK2001: unresolved external symbol _Unwind_Resume
> Display.obj : error LNK2001: unresolved external symbol _Unwind_Resume
> Surface.obj : error LNK2001: unresolved external symbol _Unwind_Resume
> Config.obj : error LNK2001: unresolved external symbol
> __gxx_personality_seh0
> Display.obj : error LNK2001: unresolved external symbol
> __gxx_personality_seh0
> Surface.obj : error LNK2001: unresolved external symbol
> __gxx_personality_seh0
>
> I'll dig in a bit to see if there's an easy fix.
>
> On Tue, Nov 28, 2017 at 11:25 PM, Martell Malone via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: martell
>> Date: Tue Nov 28 23:25:12 2017
>> New Revision: 319297
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=319297=rev
>> Log:
>> Toolchain: Normalize dwarf, sjlj and seh eh
>>
>> This is a re-apply of r319294.
>>
>> adds -fseh-exceptions and -fdwarf-exceptions flags
>>
>> clang will check if the user has specified an exception model flag,
>> in the absense of specifying the exception model clang will then check
>> the driver default and append the model flag for that target to cc1
>>
>> -fno-exceptions has a higher priority then specifying the model
>>
>> move __SEH__ macro definitions out of Targets into InitPreprocessor
>> behind the -fseh-exceptions flag
>>
>> move __ARM_DWARF_EH__ macrodefinitions out of verious targets and into
>> InitPreprocessor behind the -fdwarf-exceptions flag and arm|thumb check
>>
>> remove unused USESEHExceptions from the MinGW Driver
>>
>> fold USESjLjExceptions into a new GetExceptionModel function that
>> gives the toolchain classes more flexibility with eh models
>>
>> Reviewers: rnk, mstorsjo
>>
>> Differential Revision: https://reviews.llvm.org/D39673
>>
>> Added:
>> cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
>> Modified:
>> cfe/trunk/docs/ClangCommandLineReference.rst
>> cfe/trunk/include/clang/Basic/LangOptions.def
>> cfe/trunk/include/clang/Driver/Options.td
>> cfe/trunk/include/clang/Driver/ToolChain.h
>> cfe/trunk/lib/Basic/Targets/ARM.cpp
>> cfe/trunk/lib/Basic/Targets/OSTargets.cpp
>> cfe/trunk/lib/Basic/Targets/OSTargets.h
>> cfe/trunk/lib/Basic/Targets/X86.h
>> cfe/trunk/lib/CodeGen/BackendUtil.cpp
>> cfe/trunk/lib/CodeGen/CGException.cpp
>> cfe/trunk/lib/Driver/ToolChain.cpp
>> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>> cfe/trunk/lib/Driver/ToolChains/Darwin.h
>> cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
>> cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
>> cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
>> cfe/trunk/lib/Driver/ToolChains/MinGW.h
>> cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
>> cfe/trunk/lib/Driver/ToolChains/NetBSD.h
>> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
>> cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
>> cfe/trunk/test/Preprocessor/arm-target-features.c
>> cfe/trunk/test/Preprocessor/init.c
>>
>> Modified: cfe/trunk/docs/ClangCommandLineReference.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=319297=319296=319297=diff
>>
>> ==
>> --- cfe/trunk/docs/ClangCommandLineReference.rst (original)
>> +++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 23:25:12 2017
>> @@ -1706,10 +1706,18 @@ Which overload candidates to show when o
>>
>>  Enable C++14 sized global deallocation functions
>>
>> +.. option:: -fdwarf-exceptions
>> +
>> +Use DWARF style exceptions
>> +
>>  .. option:: -fsjlj-exceptions
>>
>>  Use SjLj style exceptions
>>
>> +.. option:: -fseh-exceptions
>> +
>> +Use SEH style exceptions
>> +
>>  .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
>>
>>  Enable the superword-level parallelism vectorization passes
>>
>> 

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp:64
+if (const UnaryOperator *U = dyn_cast(StoreE)) {
+  str = "The expression of the unary operator is an uninitialized value. "
+"The computed value will also be garbage";

xazax.hun wrote:
> lebedev.ri wrote:
> > dcoughlin wrote:
> > > "Unary operator" is probably not something we should expect users to know 
> > > about. How about just "The expression is an uninitialized value. The 
> > > computed value will also be garbage."
> > > 
> > Yep, i did not like my wording either :)
> A value being undefined does nt mean uninitialized. E.g.: the result of a bad 
> shift operation might be UndefVal as well.
> Aren't these diagnostics misleading in those cases? Or we do not care about 
> those?
I think this is for @dcoughlin to answer.
But shift operation is a binary operator, so this diff should not change that.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

[PATCH] D39279: Stringizing raw string literals containing newline

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

@jkorous-apple Thanks for the comments! Yeah, I was thinking of 
O(lenght_of_string) approach, but considering the complicatedness of the 
implementation (I guess the real implementation would be a bit more complex 
than your pseudo implementation to handle quote and '\n\r' '\r\n' cases) I 
decided to stay with O(length_of_string * number_of_endlines_in_string) but 
optimizing the number of move operations.

@vsapsai @jkorous-apple, I wonder if you can actually approve the patch or 
suggest other reviewers? Thanks!


https://reviews.llvm.org/D39279



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

r319333 - [analyzer] Fix unreachable creating PathDiagnosticLocation with widen-loops=true

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Nov 29 10:25:37 2017
New Revision: 319333

URL: http://llvm.org/viewvc/llvm-project?rev=319333=rev
Log:
[analyzer] Fix unreachable creating PathDiagnosticLocation with widen-loops=true

In the original design of the analyzer, it was assumed that a BlockEntrance
doesn't create a new binding on the Store, but this assumption isn't true when
'widen-loops' is set to true. Fix this by finding an appropriate location
BlockEntrace program points.

Patch by Henry Wong!

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

Added:
cfe/trunk/test/Analysis/loop-widening-notes.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=319333=319332=319333=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Wed Nov 29 10:25:37 
2017
@@ -690,6 +690,15 @@ PathDiagnosticLocation::create(const Pro
 return getLocationForCaller(CEE->getCalleeContext(),
 CEE->getLocationContext(),
 SMng);
+  } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (auto StmtElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
+} else if (auto NewAllocElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(
+  NewAllocElt->getAllocatorExpr()->getLocStart(), SMng);
+}
+llvm_unreachable("Unexpected CFG element at front of block");
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }

Added: cfe/trunk/test/Analysis/loop-widening-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-widening-notes.cpp?rev=319333=auto
==
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp (added)
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp Wed Nov 29 10:25:37 2017
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2 
-analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+  if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}} 
+// expected-note@-1 {{Taking true branch}}
+bar();
+  for (int i = 0;  // expected-note {{Loop condition is true.  Entering loop 
body}}
+   // expected-note@-1 {{Loop condition is false. Execution 
continues on line 16}}
+   ++i,// expected-note {{Value assigned to 'p_a'}} 
+   i < flag_a;
+   ++i) {}
+  
+  *p_a = 25609; // no-crash expected-warning {{Dereference of null pointer 
(loaded from variable 'p_a')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p_a')}}
+  return *p_a;
+}
+
+int flag_b;
+int while_analyzer_output() {
+  flag_b = 100;
+  int num = 10;
+  while (flag_b-- > 0) { // expected-note {{Loop condition is true.  Entering 
loop body}} 
+ // expected-note@-1 {{Value assigned to 'num'}} 
+ // expected-note@-2 {{Loop condition is false. 
Execution continues on line 30}}
+num = flag_b;
+  }
+  if (num < 0) // expected-note {{Assuming 'num' is >= 0}} 
+   // expected-note@-1 {{Taking false branch}}
+flag_b = 0;
+  else if (num >= 1) // expected-note {{Assuming 'num' is < 1}} 
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 50;
+  else
+flag_b = 100;
+  return flag_b / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
+
+int flag_c;
+int do_while_analyzer_output() {
+  int num = 10;
+  do {   // expected-note {{Loop condition is true. Execution continues on 
line 47}} 
+ // expected-note@-1 {{Loop condition is false.  Exiting loop}}
+num--;
+  } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}}
+  int local = 0;
+  if (num == 0)   // expected-note {{Assuming 'num' is equal to 0}} 
+  // expected-note@-1 {{Taking true branch}}
+local = 10 / num; // no-crash expected-warning {{Division by zero}}
+  // expected-note@-1 {{Division by zero}}
+  return local;
+}
+
+int flag_d;
+int test_for_loop() {
+  int num = 10;
+  for (int i = 0;// expected-note {{Loop condition is true.  Entering loop 
body}} 
+ // expected-note@-1 {{Loop condition is false. Execution 
continues on line 67}}
+   new int(10),  // expected-note {{Value assigned to 'num'}}
+   i < flag_d;
+   ++i) { 
+++num;
+  

[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

[Devin Coughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319333: [analyzer] Fix unreachable creating 
PathDiagnosticLocation with widen-loops=true (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D37187?vs=124549=124778#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37187

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  cfe/trunk/test/Analysis/loop-widening-notes.cpp


Index: cfe/trunk/test/Analysis/loop-widening-notes.cpp
===
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2 
-analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+  if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}} 
+// expected-note@-1 {{Taking true branch}}
+bar();
+  for (int i = 0;  // expected-note {{Loop condition is true.  Entering loop 
body}}
+   // expected-note@-1 {{Loop condition is false. Execution 
continues on line 16}}
+   ++i,// expected-note {{Value assigned to 'p_a'}} 
+   i < flag_a;
+   ++i) {}
+  
+  *p_a = 25609; // no-crash expected-warning {{Dereference of null pointer 
(loaded from variable 'p_a')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p_a')}}
+  return *p_a;
+}
+
+int flag_b;
+int while_analyzer_output() {
+  flag_b = 100;
+  int num = 10;
+  while (flag_b-- > 0) { // expected-note {{Loop condition is true.  Entering 
loop body}} 
+ // expected-note@-1 {{Value assigned to 'num'}} 
+ // expected-note@-2 {{Loop condition is false. 
Execution continues on line 30}}
+num = flag_b;
+  }
+  if (num < 0) // expected-note {{Assuming 'num' is >= 0}} 
+   // expected-note@-1 {{Taking false branch}}
+flag_b = 0;
+  else if (num >= 1) // expected-note {{Assuming 'num' is < 1}} 
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 50;
+  else
+flag_b = 100;
+  return flag_b / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
+
+int flag_c;
+int do_while_analyzer_output() {
+  int num = 10;
+  do {   // expected-note {{Loop condition is true. Execution continues on 
line 47}} 
+ // expected-note@-1 {{Loop condition is false.  Exiting loop}}
+num--;
+  } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}}
+  int local = 0;
+  if (num == 0)   // expected-note {{Assuming 'num' is equal to 0}} 
+  // expected-note@-1 {{Taking true branch}}
+local = 10 / num; // no-crash expected-warning {{Division by zero}}
+  // expected-note@-1 {{Division by zero}}
+  return local;
+}
+
+int flag_d;
+int test_for_loop() {
+  int num = 10;
+  for (int i = 0;// expected-note {{Loop condition is true.  Entering loop 
body}} 
+ // expected-note@-1 {{Loop condition is false. Execution 
continues on line 67}}
+   new int(10),  // expected-note {{Value assigned to 'num'}}
+   i < flag_d;
+   ++i) { 
+++num;
+  }
+  if (num == 0) // expected-note {{Assuming 'num' is equal to 0}} 
+// expected-note@-1 {{Taking true branch}}
+flag_d += 10;
+  return flag_d / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -690,6 +690,15 @@
 return getLocationForCaller(CEE->getCalleeContext(),
 CEE->getLocationContext(),
 SMng);
+  } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (auto StmtElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
+} else if (auto NewAllocElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(
+  NewAllocElt->getAllocatorExpr()->getLocStart(), SMng);
+}
+llvm_unreachable("Unexpected CFG element at front of block");
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }


Index: cfe/trunk/test/Analysis/loop-widening-notes.cpp
===
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 

[PATCH] D35894: [clangd] Code hover for Clangd

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/Protocol.h:453
+struct MarkedString {
+  /**
+   * MarkedString can be used to render human readable text. It is either a

The doc should use /// like the others


https://reviews.llvm.org/D35894



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

r319332 - [OPENMP] Do not allow `linear` clauses on non-simd distribute

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Nov 29 10:20:04 2017
New Revision: 319332

URL: http://llvm.org/viewvc/llvm-project?rev=319332=rev
Log:
[OPENMP] Do not allow `linear` clauses on non-simd distribute
directives.

`linear` clause is not allowed on non-simd distribute-based directives.

Removed:
cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_linear_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_linear_messages.cpp
Modified:
cfe/trunk/include/clang/Basic/OpenMPKinds.def
cfe/trunk/test/OpenMP/distribute_parallel_for_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_ast_print.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_ast_print.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_messages.cpp

Modified: cfe/trunk/include/clang/Basic/OpenMPKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenMPKinds.def?rev=319332=319331=319332=diff
==
--- cfe/trunk/include/clang/Basic/OpenMPKinds.def (original)
+++ cfe/trunk/include/clang/Basic/OpenMPKinds.def Wed Nov 29 10:20:04 2017
@@ -610,7 +610,6 @@ OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(sh
 OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(reduction)
 OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(copyin)
 OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(schedule)
-OPENMP_DISTRIBUTE_PARALLEL_FOR_CLAUSE(linear)
 
 // Clauses allowed for OpenMP directive 'distribute parallel for simd'
 OPENMP_DISTRIBUTE_PARALLEL_FOR_SIMD_CLAUSE(firstprivate)
@@ -746,7 +745,6 @@ OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLA
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(shared)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(reduction)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(schedule)
-OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(linear)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(num_teams)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(thread_limit)
 OPENMP_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(copyin)
@@ -807,7 +805,6 @@ OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_
 OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(num_threads)
 OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(proc_bind)
 OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(schedule)
-OPENMP_TARGET_TEAMS_DISTRIBUTE_PARALLEL_FOR_CLAUSE(linear)
 
 // Clauses allowed for OpenMP directive
 // 'target teams distribute parallel for simd'.

Removed: cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp?rev=319331=auto
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_linear_messages.cpp (removed)
@@ -1,338 +0,0 @@
-// RUN: %clang_cc1 -verify -fopenmp %s
-
-namespace X {
-  int x;
-};
-
-struct B {
-  static int ib; // expected-note {{'B::ib' declared here}}
-  static int bfoo() { return 8; }
-};
-
-int bfoo() { return 4; }
-
-int z;
-const int C1 = 1;
-const int C2 = 2;
-void test_linear_colons()
-{
-  int B = 0;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B:bfoo())
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B::ib:B:bfoo()) // expected-error 
{{unexpected ':' in nested name specifier; did you mean '::'}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B:ib) // expected-error {{use of 
undeclared identifier 'ib'; did you mean 'B::ib'}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(z:B:ib) // expected-error 
{{unexpected ':' in nested name specifier; did you mean '::'?}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B:B::bfoo())
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(X::x : ::z)
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B,::z, X::x)
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(::z)
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B::bfoo()) // expected-error 
{{expected variable name}}
-  for (int i = 0; i < 10; ++i) ;
-
-#pragma omp target
-#pragma omp teams
-#pragma omp distribute parallel for linear(B::ib,B:C1+C2)
-  for (int i = 0; i < 10; ++i) ;
-}
-
-template T test_template(T* arr, N num) {
-  N i;
-  T sum = (T)0;
-  T ind2 = - 

[PATCH] D40566: Check if MemberExpr arguments are type-dependent.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Hi, thanks for working on this!
Why don't we just set the TypeDependent bit to false when building the 
underlying MemberExpr here? Based on the section of the standard you quoted 
that node shouldn't be considered type-dependent, but it is by clang. I think 
this patch is just hiding that underlying problem by fishing out the real 
type-dependence after constructing the MemberExpr when really that information 
should be included in the MemberExpr to begin with.
Thanks,
Erik


https://reviews.llvm.org/D40566



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

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 124772.
juliehockett marked 11 inline comments as done.
juliehockett added a comment.

Moved AST matcher to ASTMatchers.h (see D40611 
), addressing comments.


https://reviews.llvm.org/D40580

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tidy/fuchsia/MultipleInheritanceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-multiple-inheritance.cpp

Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+  virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+  virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+  virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+  virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+  virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+  virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+  virtual int foo() = 0;
+  int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+  virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+  virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is disallowed [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+  virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+  virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+  virtual int foo() override { return 0; }
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+  virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+  virtual int bar() override { return 0; }
+  virtual int blat() override { return 0; }
+};
+
+int main(void) {
+  Bad_Child1 a;
+  Bad_Child2 b;
+  Bad_Child3 c;
+  Simple_Child1 d;
+  Simple_Child2 e;
+  Good_Child1 f;
+  Good_Child2 g;
+  Good_Child3 h;
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-multiple-inheritance
google-build-explicit-make-pair
google-build-namespaces
google-build-using-namespace
Index: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - fuchsia-multiple-inheritance
+
+fuchsia-multiple-inheritance
+
+
+Warns if a class inherits from multiple classes that are not pure virtual.
+
+For example, declaring a class that inherits from multiple concrete classes is
+disallowed:
+
+.. code-block:: c++
+
+  class Base_A {
+  public:
+virtual int foo() { return 0; }
+  };
+
+  class Base_B {
+  public:
+virtual int bar() { return 0; }
+  };
+
+  // Warning
+  class Bad_Child1 : public Base_A, Base_B {};
+
+A class that inherits from a pure virtual is allowed:
+
+.. code-block:: c++
+
+  class Interface_A {
+  public:
+virtual int foo() = 0;
+  };
+
+  class Interface_B {
+  public:
+virtual int bar() = 0;
+  };
+
+  // No warning
+  class Good_Child1 : public Interface_A, Interface_B {
+virtual int foo() override { return 0; }
+virtual int bar() override { return 0; }
+  };
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ 

Re: r319297 - Toolchain: Normalize dwarf, sjlj and seh eh

[Reid Kleckner via cfe-commits]

I see a bunch of link errors that suggest we're using the wrong EH
personality in MSVC mode now:

FAILED: win_clang_nacl_win64/swiftshader/libEGL.dll
win_clang_nacl_win64/swiftshader/libEGL.dll.lib
win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe
../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64
False link.exe /nologo
/IMPLIB:win_clang_nacl_win64/swiftshader/libEGL.dll.lib /DLL
/OUT:win_clang_nacl_win64/swiftshader/libEGL.dll
/PDB:win_clang_nacl_win64/swiftshader/libEGL.dll.pdb
@win_clang_nacl_win64/swiftshader/libEGL.dll.rsp
Config.obj : error LNK2001: unresolved external symbol _Unwind_Resume
Display.obj : error LNK2001: unresolved external symbol _Unwind_Resume
Surface.obj : error LNK2001: unresolved external symbol _Unwind_Resume
Config.obj : error LNK2001: unresolved external symbol
__gxx_personality_seh0
Display.obj : error LNK2001: unresolved external symbol
__gxx_personality_seh0
Surface.obj : error LNK2001: unresolved external symbol
__gxx_personality_seh0

I'll dig in a bit to see if there's an easy fix.

On Tue, Nov 28, 2017 at 11:25 PM, Martell Malone via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: martell
> Date: Tue Nov 28 23:25:12 2017
> New Revision: 319297
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319297=rev
> Log:
> Toolchain: Normalize dwarf, sjlj and seh eh
>
> This is a re-apply of r319294.
>
> adds -fseh-exceptions and -fdwarf-exceptions flags
>
> clang will check if the user has specified an exception model flag,
> in the absense of specifying the exception model clang will then check
> the driver default and append the model flag for that target to cc1
>
> -fno-exceptions has a higher priority then specifying the model
>
> move __SEH__ macro definitions out of Targets into InitPreprocessor
> behind the -fseh-exceptions flag
>
> move __ARM_DWARF_EH__ macrodefinitions out of verious targets and into
> InitPreprocessor behind the -fdwarf-exceptions flag and arm|thumb check
>
> remove unused USESEHExceptions from the MinGW Driver
>
> fold USESjLjExceptions into a new GetExceptionModel function that
> gives the toolchain classes more flexibility with eh models
>
> Reviewers: rnk, mstorsjo
>
> Differential Revision: https://reviews.llvm.org/D39673
>
> Added:
> cfe/trunk/test/CodeGenCXX/mingw-w64-exceptions.c
> Modified:
> cfe/trunk/docs/ClangCommandLineReference.rst
> cfe/trunk/include/clang/Basic/LangOptions.def
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/include/clang/Driver/ToolChain.h
> cfe/trunk/lib/Basic/Targets/ARM.cpp
> cfe/trunk/lib/Basic/Targets/OSTargets.cpp
> cfe/trunk/lib/Basic/Targets/OSTargets.h
> cfe/trunk/lib/Basic/Targets/X86.h
> cfe/trunk/lib/CodeGen/BackendUtil.cpp
> cfe/trunk/lib/CodeGen/CGException.cpp
> cfe/trunk/lib/Driver/ToolChain.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
> cfe/trunk/lib/Driver/ToolChains/Darwin.h
> cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
> cfe/trunk/lib/Driver/ToolChains/FreeBSD.h
> cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
> cfe/trunk/lib/Driver/ToolChains/MinGW.h
> cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
> cfe/trunk/lib/Driver/ToolChains/NetBSD.h
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> cfe/trunk/test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
> cfe/trunk/test/Preprocessor/arm-target-features.c
> cfe/trunk/test/Preprocessor/init.c
>
> Modified: cfe/trunk/docs/ClangCommandLineReference.rst
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/
> ClangCommandLineReference.rst?rev=319297=319296=319297=diff
> 
> ==
> --- cfe/trunk/docs/ClangCommandLineReference.rst (original)
> +++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Nov 28 23:25:12 2017
> @@ -1706,10 +1706,18 @@ Which overload candidates to show when o
>
>  Enable C++14 sized global deallocation functions
>
> +.. option:: -fdwarf-exceptions
> +
> +Use DWARF style exceptions
> +
>  .. option:: -fsjlj-exceptions
>
>  Use SjLj style exceptions
>
> +.. option:: -fseh-exceptions
> +
> +Use SEH style exceptions
> +
>  .. option:: -fslp-vectorize, -fno-slp-vectorize, -ftree-slp-vectorize
>
>  Enable the superword-level parallelism vectorization passes
>
> Modified: cfe/trunk/include/clang/Basic/LangOptions.def
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/LangOptions.def?rev=319297=319296=319297=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/LangOptions.def (original)
> +++ cfe/trunk/include/clang/Basic/LangOptions.def Tue Nov 28 23:25:12 2017
> @@ -124,7 +124,9 @@ LANGOPT(ZVector   , 1, 0, "Syste
>  LANGOPT(Exceptions, 1, 0, "exception handling")
>  

[PATCH] D40463: [analyzer] Fix false negative on post-increment of uninitialized variable.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1046
 if (V2_untested.isUnknownOrUndef()) {
   Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
+

lebedev.ri wrote:
> dcoughlin wrote:
> > Instead of generating a node node here, you'll want to generate a new state:
> > ```
> > state = state->BindExpr(U, LCtx, V2_untested);
> > ```
> > and use that state in the call to evalStore().
> > 
> > Otherwise, you'll end up exploring from both the new generated node and 
> > from *I.
> > 
> > Here is a test that demonstrates why this is a problem. You should add it 
> > to your tests.
> > 
> > ```
> > void foo() {
> >   int x;
> > 
> >   x++; // expected-warning {{The expression of the unary operator is an 
> > uninitialized value. The computed value will also be garbage}}
> > 
> >   clang_analyzer_warnIfReached(); // no-warning
> > 
> > ```
> > The undefined assignment checker generates what we call "fatal" errors. 
> > These errors are so bad that it decides not to explore further on that path 
> > to report additional errors. This means that the analyzer should treat any 
> > code that is dominated by such an error as not reachable. You'll need to 
> > add the `debug.ExprInspection` checker to your test RUN line and a 
> > prototype for clang_analyzer_warnIfReached() for this test to to work.
> > 
> > Instead of generating a node node here, you'll want to generate a new state:
> > ```
> > state = state->BindExpr(U, LCtx, V2_untested);
> > ```
> > and use that state in the call to evalStore().
> Hm, so the only change needed here is
> ```diff
> - Bldr.generateNode(U, *I, state->BindExpr(U, LCtx, V2_untested));
> + state = state->BindExpr(U, LCtx, V2_untested);
> ```
> ?
Yes, that's right. What you have here looks good to me.



Comment at: test/Analysis/malloc-plist.c:13
 int *p = malloc(12);
-(*p)++;
+(*p)++; // expected-warning {{The expression of the unary operator is 
an uninitialized value. The computed value will also be garbage}}
+*p = 0; // no-warning

lebedev.ri wrote:
> dcoughlin wrote:
> > Once you change the core modeling to not generate an extra node, you'll 
> > want to initialize `*p` to `0` or something to preserve the intent of this 
> > test. For this test it is important that the increment not stop further 
> > exploration of the path.
> Hmm, *this* test did not break after i changed 
> `ExprEngine::VisitIncrementDecrementOperator()`.
> Did i change it wrong?
Aah, my mistake. I though this test was passing -verify (which verifies 
expected-warning{{}} contents). Instead, it is only checking the plists, so the 
missing expected leak would only be caught by the plist change.

I suggest changing the test to:
```
...
if (in > 5) {
int *p = malloc(12);
*p = 0;
(*p)++;
}
...
```

So that the test doesn't generate a fatal error for the access to uninitialized 
memory and instead continues to check for the path of the leak.



Comment at: test/Analysis/malloc-plist.c:112
+(*p)++; // expected-warning {{The expression is an uninitialized value. 
The computed value will also be garbage}}
+*p = 0; // no-warning
+(*p)++; // no-warning

Here you should stick the `*p = 0;` before the post increment so the rest of 
the code is exercised.


Repository:
  rC Clang

https://reviews.llvm.org/D40463



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

[PATCH] D39376: [PowerPC] Add implementation for -msave-toc-indirect option - clang portion

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: include/clang/Driver/Options.td:1907
 def mvsx : Flag<["-"], "mvsx">, Group;
+def msave_toc_indirect : Flag<["-"], "msave-toc-indirect">, 
Group;
 def mno_vsx : Flag<["-"], "mno-vsx">, Group;

hfinkel wrote:
> You also need mno_save_toc_indirect here too.
nit: The new option splits the mvsx and mno_vsx pair. I think we should keep 
the related options together. 


https://reviews.llvm.org/D39376



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

[PATCH] D40611: Add has definition AST matcher

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
Herald added a subscriber: klimek.

Adds AST matcher for the definition of a CXXRecordDecl.


https://reviews.llvm.org/D40611

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2008,5 +2008,26 @@
   cxxNewExpr(hasArraySize(integerLiteral(equals(10));
 }
 
+TEST(HasDefinition, MatchesStructDefinition) {
+  EXPECT_TRUE(matches("struct x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("struct x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesClassDefinition) {
+  EXPECT_TRUE(matches("class x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("class x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesUnionDefinition) {
+  EXPECT_TRUE(matches("union x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("union x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -250,6 +250,7 @@
   REGISTER_MATCHER(hasDeclContext);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
+  REGISTER_MATCHER(hasDefinition);
   REGISTER_MATCHER(hasDescendant);
   REGISTER_MATCHER(hasDestinationType);
   REGISTER_MATCHER(hasDynamicExceptionSpec);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -5853,6 +5853,17 @@
 InnerMatcher.matches(*Node.getArraySize(), Finder, Builder);
 }
 
+/// \brief Matches a declaration that is defined.
+///
+/// Example matches x (matcher = cxxRecordDecl(hasDefinition()))
+/// \code
+/// class x {};
+/// class y;
+/// \endcode
+AST_MATCHER(CXXRecordDecl, hasDefinition) {
+  return Node.hasDefinition();
+}
+
 } // namespace ast_matchers
 } // namespace clang
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -2307,6 +2307,15 @@
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclhasDefinition
+Matches a declaration 
that is defined.
+
+Example matches x (matcher = cxxRecordDecl(hasDefinition()))
+class x {};
+class y;
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclisDerivedFromstd::string 
BaseName
 Overloaded method as 
shortcut for isDerivedFrom(hasName(...)).
 


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2008,5 +2008,26 @@
   cxxNewExpr(hasArraySize(integerLiteral(equals(10));
 }
 
+TEST(HasDefinition, MatchesStructDefinition) {
+  EXPECT_TRUE(matches("struct x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("struct x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesClassDefinition) {
+  EXPECT_TRUE(matches("class x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("class x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
+TEST(HasDefinition, MatchesUnionDefinition) {
+  EXPECT_TRUE(matches("union x {};",
+  cxxRecordDecl(hasDefinition(;
+  EXPECT_TRUE(notMatches("union x;",
+  cxxRecordDecl(hasDefinition(;
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -250,6 +250,7 @@
   REGISTER_MATCHER(hasDeclContext);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
+  REGISTER_MATCHER(hasDefinition);
   REGISTER_MATCHER(hasDescendant);
   REGISTER_MATCHER(hasDestinationType);
   REGISTER_MATCHER(hasDynamicExceptionSpec);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++