Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

ParmVarDecl corresponding to unnamed parameter incorrectly has a name #29140

Open Quuxplusone opened 8 years ago

Quuxplusone commented 8 years ago
Bugzilla Link PR29145
Status NEW
Importance P normal
Reported by Lukasz Anforowicz (lukasza@chromium.org)
Reported on 2016-08-25 18:21:43 -0700
Last modified on 2016-09-06 13:04:03 -0700
Version trunk
Hardware PC Linux
CC llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
clang version 4.0.0 (trunk 277962)

Expected behavior:

    1. The ParmVarDecl corresponding to
           void f(int);
       declaration on line 5 should have
       1) no identifier
       2) no name

    2. I can safely generate replacements of text inside parm.getLocation()
       (e.g. rename "dataSize" to "data_size").

Actual behavior:

    1. There are 2 ParmVarDecls on line 5.
       One of these ParmVarDecls has a non-empty name and identifier.

    2. Generating replacements based on parm.getLocation() would
       butcher the source code.

    3. Output of the repro program below:

        Name = ''
        Identifier = '0x0'            <-- looking good so far...
        Line = 'input.cc:5:19'
        ---------------------------------
        Name = 'dataSize'             <-- THIS IS THE BUG
        Identifier = '0x3a05f98'
        Line = 'input.cc:5:19'
        ---------------------------------
        Name = ''
        Identifier = '0x0'
        Line = 'input.cc:3:13'        <- please ignore ParmVarDecls from other lines
        ---------------------------------
        Name = 'dataSize'
        Identifier = '0x3a05f98'
        Line = 'input.cc:9:28'        <- please ignore ParmVarDecls from other lines

Repro:

#include <memory>

#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/ASTMatchers/ASTMatchersMacros.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Refactoring.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/TargetSelect.h"

using namespace clang::ast_matchers;

class ReproMatchCallback : public MatchFinder::MatchCallback {
 public:
  void run(const MatchFinder::MatchResult& result) override {
    const clang::ParmVarDecl* parm = result.Nodes.getNodeAs<clang::ParmVarDecl>("decl");
    if (!parm)
      return;

    llvm::outs() << "---------------------------------\n";
    llvm::outs() << "Name = '" << parm->getName() << "'\n";
    llvm::outs() << "Identifier = '" << parm->getIdentifier() << "'\n";
    llvm::outs() << "Line = '" << parm->getLocation().printToString(result.Context->getSourceManager()) << "'\n";
  }
};

int main(int argc, const char* argv[]) {
  llvm::InitializeNativeTarget();
  llvm::InitializeNativeTargetAsmParser();
  llvm::cl::OptionCategory category("Repro.");
  clang::tooling::CommonOptionsParser options(argc, argv, category);
  clang::tooling::ClangTool tool(options.getCompilations(),
                                 options.getSourcePathList());

  const char* code = R"code(            // line 1
      template <typename T>             // line 2
      class Class {                     // line 3
       public:                          // line 4
        void f(int);  // <- NO NAME!    // line 5
      };                                // line 6
                                        // line 7
      template <typename T>             // line 8
      void Class<T>::f(int dataSize){}; // line 9
                                        // line 10
      void foo() {                      // line 11
        Class<char>().f(123);           // line 12
      };                                // line 13
  )code";

  auto matcher = id("decl", parmVarDecl());

  MatchFinder match_finder;
  ReproMatchCallback match_callback;
  match_finder.addMatcher(matcher, &match_callback);
  std::unique_ptr<clang::tooling::FrontendActionFactory> factory =
      clang::tooling::newFrontendActionFactory(&match_finder);
  clang::tooling::runToolOnCode(factory->create(), code);
  return 0;
}
Quuxplusone commented 8 years ago

The bad thing is happening in addInstantiatedParametersToScope in SemaTemplateInstantiateDecl.cpp. When we update the names and types of the ParmVarDecls of the function to refer to the definition rather than the definition, we should update the source locations (both the name location and the TypeSourceInfo) as well.

Quuxplusone commented 8 years ago

Fix proposal is getting reviewed at https://reviews.llvm.org/D24268